Attachment 358364 Details for Bug 188248 – WIP

[patch] WIP

bug-188248-20190104140606.patch (text/plain), 7.00 KB, created by youenn fablet on 2019-01-04 14:06:07 PST

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-01-04 14:06:07 PST

Size: 7.00 KB

patch

obsolete

>Subversion Revision: 239617
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 73dc24d80638562dfe4962e7577ee4ac6863b0af..d6088df8b09f52284d62bf700b4bbdb8776fda95 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,23 @@
>+2019-01-04  Youenn Fablet  <youenn@apple.com>
>+
>+        service worker fetch handler results in bad referrer
>+        https://bugs.webkit.org/show_bug.cgi?id=188248
>+        <rdar://problem/47050478>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Response sanitization was removing the ReferrerPolicy header from opaque redirect responses.
>+        Reduce sanitization of opaque redirect responses to opaque responses and allow Location header.
>+        Make sure referrer policy is updated for all load redirections, not only CORS loads.
>+
>+        Covered by updated test.
>+
>+        * loader/SubresourceLoader.cpp:
>+        (WebCore::SubresourceLoader::checkRedirectionCrossOriginAccessControl):
>+        * platform/network/ResourceResponseBase.cpp:
>+        (WebCore::isSafeCrossOriginResponseHeader):
>+        (WebCore::ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting):
>+
> 2019-01-04  Youenn Fablet  <youenn@apple.com>
> 
>         Service Worker fetch should obey its referrer policy
>diff --git a/Source/WebCore/loader/SubresourceLoader.cpp b/Source/WebCore/loader/SubresourceLoader.cpp
>index b301bb9bf6f7ade6fcdb68219e804f32e0c76b8e..52e919b2b5a21dac28dfc247c444695e8ff06e19 100644
>--- a/Source/WebCore/loader/SubresourceLoader.cpp
>+++ b/Source/WebCore/loader/SubresourceLoader.cpp
>@@ -567,19 +567,18 @@ bool SubresourceLoader::checkRedirectionCrossOriginAccessControl(const ResourceR
> 
>     ASSERT(options().mode != FetchOptions::Mode::SameOrigin || !m_resource->isCrossOrigin());
> 
>-    if (options().mode != FetchOptions::Mode::Cors)
>-        return true;
>+    if (options().mode == FetchOptions::Mode::Cors) {
>+        // Implementing https://fetch.spec.whatwg.org/#concept-http-redirect-fetch step 8 & 9.
>+        if (m_resource->isCrossOrigin() && !isValidCrossOriginRedirectionURL(newRequest.url())) {
>+            errorMessage = "URL is either a non-HTTP URL or contains credentials."_s;
>+            return false;
>+        }
> 
>-    // Implementing https://fetch.spec.whatwg.org/#concept-http-redirect-fetch step 8 & 9.
>-    if (m_resource->isCrossOrigin() && !isValidCrossOriginRedirectionURL(newRequest.url())) {
>-        errorMessage = "URL is either a non-HTTP URL or contains credentials."_s;
>-        return false;
>+        ASSERT(m_origin);
>+        if (crossOriginFlag && !passesAccessControlCheck(redirectResponse, options().storedCredentialsPolicy, *m_origin, errorMessage))
>+            return false;
>     }
> 
>-    ASSERT(m_origin);
>-    if (crossOriginFlag && !passesAccessControlCheck(redirectResponse, options().storedCredentialsPolicy, *m_origin, errorMessage))
>-        return false;
>-
>     bool redirectingToNewOrigin = false;
>     if (m_resource->isCrossOrigin()) {
>         if (!crossOriginFlag && isNextRequestCrossOrigin)
>@@ -594,7 +593,7 @@ bool SubresourceLoader::checkRedirectionCrossOriginAccessControl(const ResourceR
> 
>     updateReferrerPolicy(redirectResponse.httpHeaderField(HTTPHeaderName::ReferrerPolicy));
>     
>-    if (redirectingToNewOrigin) {
>+    if (options().mode == FetchOptions::Mode::Cors && redirectingToNewOrigin) {
>         cleanHTTPRequestHeadersForAccessControl(newRequest, options().httpHeadersToKeep);
>         updateRequestForAccessControl(newRequest, *m_origin, options().storedCredentialsPolicy);
>     }
>diff --git a/Source/WebCore/platform/network/ResourceResponseBase.cpp b/Source/WebCore/platform/network/ResourceResponseBase.cpp
>index c8909755e55ec48f11eed63c8a4e218fa2d01492..20ac64d57034c69ea92bc7a43536f16d795ffa29 100644
>--- a/Source/WebCore/platform/network/ResourceResponseBase.cpp
>+++ b/Source/WebCore/platform/network/ResourceResponseBase.cpp
>@@ -401,6 +401,7 @@ static bool isSafeCrossOriginResponseHeader(HTTPHeaderName name)
>         || name == HTTPHeaderName::LastEventID
>         || name == HTTPHeaderName::LastModified
>         || name == HTTPHeaderName::Link
>+        || name == HTTPHeaderName::Location
>         || name == HTTPHeaderName::Pragma
>         || name == HTTPHeaderName::Range
>         || name == HTTPHeaderName::ReferrerPolicy
>@@ -441,7 +442,8 @@ void ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting()
>         m_httpHeaderFields = WTFMove(filteredHeaders);
>         return;
>     }
>-    case ResourceResponse::Tainting::Opaque: {
>+    case ResourceResponse::Tainting::Opaque:
>+    case ResourceResponse::Tainting::Opaqueredirect: {
>         HTTPHeaderMap filteredHeaders;
>         for (auto& header : m_httpHeaderFields.commonHeaders()) {
>             if (isSafeCrossOriginResponseHeader(header.key))
>@@ -450,11 +452,6 @@ void ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting()
>         m_httpHeaderFields = WTFMove(filteredHeaders);
>         return;
>     }
>-    case ResourceResponse::Tainting::Opaqueredirect: {
>-        auto location = httpHeaderField(HTTPHeaderName::Location);
>-        m_httpHeaderFields.clear();
>-        m_httpHeaderFields.add(HTTPHeaderName::Location, WTFMove(location));
>-    }
>     }
> }
> 
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index 59dfeab12d1fdf2093c509c41beb161a598eb1ba..d7eb69eda7325ea4f5fb65b579d73f88e00382e7 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-01-04  Youenn Fablet  <youenn@apple.com>
>+
>+        service worker fetch handler results in bad referrer
>+        https://bugs.webkit.org/show_bug.cgi?id=188248
>+        <rdar://problem/47050478>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt:
>+
> 2019-01-04  Youenn Fablet  <youenn@apple.com>
> 
>         Service Worker fetch should obey its referrer policy
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt
>index 2ea4aded04f513ea08ac60820891f62bf0390210..a4f692904444ee0a7555f83d516ce48adaad1031 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/referrer-policy-header.https-expected.txt
>@@ -1,7 +1,6 @@
> 
>-
> PASS Initialize global state (service worker registration) 
>-FAIL Referrer for a main resource redirected with referrer-policy (origin) should only have origin. assert_equals: expected "https://localhost:9443/" but got "https://localhost:9443/service-workers/service-worker/referrer-policy-header.https.html"
>+PASS Referrer for a main resource redirected with referrer-policy (origin) should only have origin. 
> PASS Referrer for fetch requests initiated from a service worker with referrer-policy (origin) should only have origin. 
> PASS Remove registration as a cleanup 
>

Actions: View | Formatted Diff | Diff

Attachments on bug 188248: 358364 | 358386 | 358395 | 358402 | 358410 | 358416 | 358419 | 358424 | 358428 | 358429 | 358452 | 358454