Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (206857 => 206858)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2016-10-06 09:43:52 UTC (rev 206857)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2016-10-06 10:00:28 UTC (rev 206858)
@@ -1,5 +1,15 @@
2016-10-06 Youenn Fablet <you...@apple.com>
+ [Fetch API] Forbid redirection to non-HTTP(s) URL in non-navigation mode.
+ https://bugs.webkit.org/show_bug.cgi?id=162785
+
+ Reviewed by Alex Christensen.
+
+ * web-platform-tests/fetch/api/redirect/redirect-to-dataurl-expected.txt:
+ * web-platform-tests/fetch/api/redirect/redirect-to-dataurl-worker-expected.txt:
+
+2016-10-06 Youenn Fablet <you...@apple.com>
+
[WK2] 304 revalidation on the network process does not update the validated response
https://bugs.webkit.org/show_bug.cgi?id=162973
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/redirect/redirect-to-dataurl-expected.txt (206857 => 206858)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/redirect/redirect-to-dataurl-expected.txt 2016-10-06 09:43:52 UTC (rev 206857)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/redirect/redirect-to-dataurl-expected.txt 2016-10-06 10:00:28 UTC (rev 206858)
@@ -4,8 +4,8 @@
CONSOLE MESSAGE: Cross-origin redirection to data:text/plain;base64,cmVzcG9uc2UncyBib2R5 denied by Cross-Origin Resource Sharing policy: URL is either a non-HTTP URL or contains credentials.
PASS Testing data URL loading after same-origin redirection (cors mode)
-FAIL Testing data URL loading after same-origin redirection (no-cors mode) assert_unreached: Should have rejected. Reached unreachable code
+PASS Testing data URL loading after same-origin redirection (no-cors mode)
PASS Testing data URL loading after same-origin redirection (same-origin mode)
PASS Testing data URL loading after cross-origin redirection (cors mode)
-FAIL Testing data URL loading after cross-origin redirection (no-cors mode) assert_unreached: Should have rejected. Reached unreachable code
+PASS Testing data URL loading after cross-origin redirection (no-cors mode)
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/redirect/redirect-to-dataurl-worker-expected.txt (206857 => 206858)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/redirect/redirect-to-dataurl-worker-expected.txt 2016-10-06 09:43:52 UTC (rev 206857)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/redirect/redirect-to-dataurl-worker-expected.txt 2016-10-06 10:00:28 UTC (rev 206858)
@@ -4,8 +4,8 @@
CONSOLE MESSAGE: Cross-origin redirection to data:text/plain;base64,cmVzcG9uc2UncyBib2R5 denied by Cross-Origin Resource Sharing policy: URL is either a non-HTTP URL or contains credentials.
PASS Testing data URL loading after same-origin redirection (cors mode)
-FAIL Testing data URL loading after same-origin redirection (no-cors mode) assert_unreached: Should have rejected. Reached unreachable code
+PASS Testing data URL loading after same-origin redirection (no-cors mode)
PASS Testing data URL loading after same-origin redirection (same-origin mode)
PASS Testing data URL loading after cross-origin redirection (cors mode)
-FAIL Testing data URL loading after cross-origin redirection (no-cors mode) assert_unreached: Should have rejected. Reached unreachable code
+PASS Testing data URL loading after cross-origin redirection (no-cors mode)
Modified: trunk/Source/WebCore/ChangeLog (206857 => 206858)
--- trunk/Source/WebCore/ChangeLog 2016-10-06 09:43:52 UTC (rev 206857)
+++ trunk/Source/WebCore/ChangeLog 2016-10-06 10:00:28 UTC (rev 206858)
@@ -1,5 +1,38 @@
2016-10-06 Youenn Fablet <you...@apple.com>
+ [Fetch API] Forbid redirection to non-HTTP(s) URL in non-navigation mode.
+ https://bugs.webkit.org/show_bug.cgi?id=162785
+
+ Reviewed by Alex Christensen.
+
+ Covered by rebased and existing tests.
+
+ Ensuring non-HTTP redirection URLs are not followed at DocumentThreadableLoader level for fetch API only.
+ This should be applied to all clients at some point, but there is still some uncertainty for data URLs.
+
+ Did some refactoring to better separate the case of security checks in case of regular request or redirected request.
+ This allows in particular to handle more clearly the case of data URLs which are allowed in all modes for regular requests.
+ But they are not allowed for same-origin redirected requests.
+
+ * WebCore.xcodeproj/project.pbxproj:
+ * loader/DocumentThreadableLoader.cpp:
+ (WebCore::reportRedirectionWithBadScheme): Reporting bad scheme redirection error.
+ (WebCore::DocumentThreadableLoader::redirectReceived): Checking that redirection URLs are HTTP(s) in case of Fetch API.
+ * loader/SubresourceLoader.cpp:
+ (WebCore::SubresourceLoader::willSendRequestInternal):
+ * loader/cache/CachedResourceLoader.cpp:
+ (WebCore::CachedResourceLoader::requestImage):
+ (WebCore::CachedResourceLoader::checkInsecureContent):
+ (WebCore::CachedResourceLoader::allowedByContentSecurityPolicy):
+ (WebCore::isSameOriginDataURL):
+ (WebCore::CachedResourceLoader::canRequest):
+ (WebCore::CachedResourceLoader::canRequestAfterRedirection):
+ (WebCore::CachedResourceLoader::canRequestInContentDispositionAttachmentSandbox):
+ (WebCore::CachedResourceLoader::requestResource):
+ * loader/cache/CachedResourceLoader.h:
+
+2016-10-06 Youenn Fablet <you...@apple.com>
+
[Fetch API] Use ReadableStream pull to transfer binary data to stream when application needs it
https://bugs.webkit.org/show_bug.cgi?id=162892
Modified: trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj (206857 => 206858)
--- trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj 2016-10-06 09:43:52 UTC (rev 206857)
+++ trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj 2016-10-06 10:00:28 UTC (rev 206858)
@@ -5795,7 +5795,7 @@
CE2849871CA360DF00B4A57F /* ContentSecurityPolicyDirectiveNames.h in Headers */ = {isa = PBXBuildFile; fileRef = CE2849861CA360DF00B4A57F /* ContentSecurityPolicyDirectiveNames.h */; };
CE2849891CA3614600B4A57F /* ContentSecurityPolicyDirectiveNames.cpp in Sources */ = {isa = PBXBuildFile; fileRef = CE2849881CA3614600B4A57F /* ContentSecurityPolicyDirectiveNames.cpp */; };
CE6DADF91C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.cpp in Sources */ = {isa = PBXBuildFile; fileRef = CE6DADF71C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.cpp */; };
- CE6DADFA1C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.h in Headers */ = {isa = PBXBuildFile; fileRef = CE6DADF81C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.h */; };
+ CE6DADFA1C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.h in Headers */ = {isa = PBXBuildFile; fileRef = CE6DADF81C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.h */; settings = {ATTRIBUTES = (Private, ); }; };
CE799F971C6A46BC0097B518 /* ContentSecurityPolicySourceList.cpp in Sources */ = {isa = PBXBuildFile; fileRef = CE799F951C6A46BC0097B518 /* ContentSecurityPolicySourceList.cpp */; };
CE799F981C6A46BC0097B518 /* ContentSecurityPolicySourceList.h in Headers */ = {isa = PBXBuildFile; fileRef = CE799F961C6A46BC0097B518 /* ContentSecurityPolicySourceList.h */; };
CE799F9B1C6A4BCD0097B518 /* ContentSecurityPolicyDirectiveList.cpp in Sources */ = {isa = PBXBuildFile; fileRef = CE799F991C6A4BCD0097B518 /* ContentSecurityPolicyDirectiveList.cpp */; };
Modified: trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp (206857 => 206858)
--- trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp 2016-10-06 09:43:52 UTC (rev 206857)
+++ trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp 2016-10-06 10:00:28 UTC (rev 206858)
@@ -216,6 +216,11 @@
client.didFail(ResourceError(errorDomainWebKitInternal, 0, url, "Cross-origin redirection denied by Cross-Origin Resource Sharing policy.", ResourceError::Type::AccessControl));
}
+static inline void reportRedirectionWithBadScheme(ThreadableLoaderClient& client, const URL& url)
+{
+ client.didFail(ResourceError(errorDomainWebKitInternal, 0, url, "Redirection to URL with a scheme that is not HTTP(S).", ResourceError::Type::AccessControl));
+}
+
void DocumentThreadableLoader::redirectReceived(CachedResource* resource, ResourceRequest& request, const ResourceResponse& redirectResponse)
{
ASSERT(m_client);
@@ -222,6 +227,16 @@
ASSERT_UNUSED(resource, resource == m_resource);
Ref<DocumentThreadableLoader> protectedThis(*this);
+
+ // FIXME: We restrict this check to Fetch API for the moment, as this might disrupt WorkerScriptLoader.
+ // Reassess this check based on https://github.com/whatwg/fetch/issues/393 discussions.
+ // We should also disable that check in navigation mode.
+ if (!request.url().protocolIsInHTTPFamily() && m_options.initiator == cachedResourceRequestInitiators().fetch) {
+ reportRedirectionWithBadScheme(*m_client, request.url());
+ clearResource();
+ return;
+ }
+
if (!isAllowedByContentSecurityPolicy(request.url(), redirectResponse.isNull() ? ContentSecurityPolicy::RedirectResponseReceived::No : ContentSecurityPolicy::RedirectResponseReceived::Yes)) {
reportContentSecurityPolicyError(*m_client, redirectResponse.url());
clearResource();
Modified: trunk/Source/WebCore/loader/SubresourceLoader.cpp (206857 => 206858)
--- trunk/Source/WebCore/loader/SubresourceLoader.cpp 2016-10-06 09:43:52 UTC (rev 206857)
+++ trunk/Source/WebCore/loader/SubresourceLoader.cpp 2016-10-06 10:00:28 UTC (rev 206858)
@@ -200,7 +200,7 @@
m_frame->page()->diagnosticLoggingClient().logDiagnosticMessageWithResult(DiagnosticLoggingKeys::cachedResourceRevalidationKey(), emptyString(), DiagnosticLoggingResultFail, ShouldSample::Yes);
}
- if (!m_documentLoader->cachedResourceLoader().canRequest(m_resource->type(), newRequest.url(), options(), false /* forPreload */, true /* didReceiveRedirectResponse */)) {
+ if (!m_documentLoader->cachedResourceLoader().canRequestAfterRedirection(m_resource->type(), newRequest.url(), options())) {
cancel();
return;
}
Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp (206857 => 206858)
--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2016-10-06 09:43:52 UTC (rev 206857)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp 2016-10-06 10:00:28 UTC (rev 206858)
@@ -183,7 +183,7 @@
if (Document* document = frame->document())
document->contentSecurityPolicy()->upgradeInsecureRequestIfNeeded(request.mutableResourceRequest(), ContentSecurityPolicy::InsecureRequestType::Load);
URL requestURL = request.resourceRequest().url();
- if (requestURL.isValid() && canRequest(CachedResource::ImageResource, requestURL, request.options(), request.forPreload()))
+ if (requestURL.isValid() && canRequest(CachedResource::ImageResource, requestURL, request))
PingLoader::loadImage(*frame, requestURL);
return nullptr;
}
@@ -333,6 +333,10 @@
bool CachedResourceLoader::checkInsecureContent(CachedResource::Type type, const URL& url) const
{
+
+ if (!canRequestInContentDispositionAttachmentSandbox(type, url))
+ return false;
+
switch (type) {
case CachedResource::Script:
#if ENABLE(XSLT)
@@ -379,13 +383,8 @@
return true;
}
-static inline bool isSameOriginDataURL(const URL& url, const ResourceLoaderOptions& options, bool didReceiveRedirectResponse)
+bool CachedResourceLoader::allowedByContentSecurityPolicy(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, ContentSecurityPolicy::RedirectResponseReceived redirectResponseReceived)
{
- return !didReceiveRedirectResponse && url.protocolIsData() && options.sameOriginDataURLFlag == SameOriginDataURLFlag::Set;
-}
-
-bool CachedResourceLoader::allowedByContentSecurityPolicy(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, bool didReceiveRedirectResponse)
-{
if (options.contentSecurityPolicyImposition == ContentSecurityPolicyImposition::SkipPolicyCheck)
return true;
@@ -392,8 +391,6 @@
ASSERT(m_document);
ASSERT(m_document->contentSecurityPolicy());
- auto redirectResponseReceived = didReceiveRedirectResponse ? ContentSecurityPolicy::RedirectResponseReceived::Yes : ContentSecurityPolicy::RedirectResponseReceived::No;
-
switch (type) {
#if ENABLE(XSLT)
case CachedResource::XSLStyleSheet:
@@ -430,25 +427,33 @@
default:
ASSERT_NOT_REACHED();
}
+
return true;
}
-bool CachedResourceLoader::canRequest(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, bool forPreload, bool didReceiveRedirectResponse)
+static inline bool isSameOriginDataURL(const URL& url, const ResourceLoaderOptions& options)
{
+ // FIXME: Remove same-origin data URL flag since it was removed from fetch spec (https://github.com/whatwg/fetch/issues/381).
+ return url.protocolIsData() && options.sameOriginDataURLFlag == SameOriginDataURLFlag::Set;
+}
+
+bool CachedResourceLoader::canRequest(CachedResource::Type type, const URL& url, const CachedResourceRequest& request)
+{
+ auto& options = request.options();
+
if (document() && !document()->securityOrigin()->canDisplay(url)) {
- if (!forPreload)
+ if (!request.forPreload())
FrameLoader::reportLocalLoadFailed(frame(), url.stringCenterEllipsizedToLength());
LOG(ResourceLoading, "CachedResourceLoader::requestResource URL was not allowed by SecurityOrigin::canDisplay");
return false;
}
- // FIXME: Remove same-origin data URL flag since it was removed from fetch spec (see https://github.com/whatwg/fetch/issues/381).
- if (options.mode == FetchOptions::Mode::SameOrigin && !isSameOriginDataURL(url, options, didReceiveRedirectResponse) && !m_document->securityOrigin()->canRequest(url)) {
+ if (options.mode == FetchOptions::Mode::SameOrigin && !m_document->securityOrigin()->canRequest(url) && !isSameOriginDataURL(url, options)) {
printAccessDeniedMessage(url);
return false;
}
- if (!allowedByContentSecurityPolicy(type, url, options, didReceiveRedirectResponse))
+ if (!allowedByContentSecurityPolicy(type, url, options, ContentSecurityPolicy::RedirectResponseReceived::No))
return false;
// SVG Images have unique security rules that prevent all subresource requests except for data urls.
@@ -457,14 +462,38 @@
return false;
}
- if (!canRequestInContentDispositionAttachmentSandbox(type, url))
+ // Last of all, check for insecure content. We do this last so that when folks block insecure content with a CSP policy, they don't get a warning.
+ // They'll still get a warning in the console about CSP blocking the load.
+
+ // FIXME: Should we consider whether the request is for preload here?
+ if (!checkInsecureContent(type, url))
return false;
- // Last of all, check for insecure content. We do this last so that when
- // folks block insecure content with a CSP policy, they don't get a warning.
+ return true;
+}
+
+// FIXME: Should we find a way to know whether the redirection is for a preload request like we do for CachedResourceLoader::canRequest?
+bool CachedResourceLoader::canRequestAfterRedirection(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options)
+{
+ if (document() && !document()->securityOrigin()->canDisplay(url)) {
+ FrameLoader::reportLocalLoadFailed(frame(), url.stringCenterEllipsizedToLength());
+ LOG(ResourceLoading, "CachedResourceLoader::requestResource URL was not allowed by SecurityOrigin::canDisplay");
+ return false;
+ }
+
+ // FIXME: According to https://fetch.spec.whatwg.org/#http-redirect-fetch, we should check that the URL is HTTP(s) except if in navigation mode.
+ // But we currently allow at least data URLs to be loaded.
+
+ if (options.mode == FetchOptions::Mode::SameOrigin && !m_document->securityOrigin()->canRequest(url)) {
+ printAccessDeniedMessage(url);
+ return false;
+ }
+
+ if (!allowedByContentSecurityPolicy(type, url, options, ContentSecurityPolicy::RedirectResponseReceived::Yes))
+ return false;
+
+ // Last of all, check for insecure content. We do this last so that when folks block insecure content with a CSP policy, they don't get a warning.
// They'll still get a warning in the console about CSP blocking the load.
-
- // FIXME: Should we consider forPreload here?
if (!checkInsecureContent(type, url))
return false;
@@ -474,7 +503,7 @@
bool CachedResourceLoader::canRequestInContentDispositionAttachmentSandbox(CachedResource::Type type, const URL& url) const
{
Document* document;
-
+
// FIXME: Do we want to expand this to all resource types that the mixed content checker would consider active content?
switch (type) {
case CachedResource::MainResource:
@@ -624,7 +653,8 @@
prepareFetch(type, request);
- if (!canRequest(type, url, request.options(), request.forPreload())) {
+ // We are passing url as well as request, as request url may contain a fragment identifier.
+ if (!canRequest(type, url, request)) {
RELEASE_LOG_IF_ALLOWED("requestResource: Not allowed to request resource (frame = %p)", frame());
return nullptr;
}
Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.h (206857 => 206858)
--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.h 2016-10-06 09:43:52 UTC (rev 206857)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.h 2016-10-06 10:00:28 UTC (rev 206858)
@@ -30,6 +30,7 @@
#include "CachedResource.h"
#include "CachedResourceHandle.h"
#include "CachedResourceRequest.h"
+#include "ContentSecurityPolicy.h"
#include "ResourceLoadPriority.h"
#include "ResourceTimingInformation.h"
#include "Timer.h"
@@ -122,7 +123,7 @@
void loadDone(CachedResource*, bool shouldPerformPostLoadActions = true);
WEBCORE_EXPORT void garbageCollectDocumentResources();
-
+
void incrementRequestCount(const CachedResource&);
void decrementRequestCount(const CachedResource&);
int requestCount() const { return m_requestCount; }
@@ -135,7 +136,8 @@
void checkForPendingPreloads();
void printPreloadStats();
- bool canRequest(CachedResource::Type, const URL&, const ResourceLoaderOptions&, bool forPreload = false, bool didReceiveRedirectResponse = false);
+ bool canRequest(CachedResource::Type, const URL&, const CachedResourceRequest&);
+ bool canRequestAfterRedirection(CachedResource::Type, const URL&, const ResourceLoaderOptions&);
static const ResourceLoaderOptions& defaultCachedResourceOptions();
@@ -164,7 +166,7 @@
bool shouldContinueAfterNotifyingLoadedFromMemoryCache(const CachedResourceRequest&, CachedResource*);
bool checkInsecureContent(CachedResource::Type, const URL&) const;
- bool allowedByContentSecurityPolicy(CachedResource::Type, const URL&, const ResourceLoaderOptions&, bool);
+ bool allowedByContentSecurityPolicy(CachedResource::Type, const URL&, const ResourceLoaderOptions&, ContentSecurityPolicy::RedirectResponseReceived);
void performPostLoadActions();
