New issue
Advanced search Search tips

Issue 628457 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Referrers reset incorrectly when following CORS-enabled redirects

Project Member Reported by est...@chromium.org, Jul 15 2016

Issue description

Chrome currently fails the web platform test referrer-policy/origin-when-cross-origin/http-rp/cross-origin/http-http/fetch-request/cross-origin.swap-origin-redirect.http.html (and others for the same reason).

This test sets a document referrer policy of origin-when-cross-origin and then issues a fetch for a cross-origin resource which redirects back to the origin of the page. The test expects the final referrer (the referrer sent while following the redirect) to be the origin, not the full URL, even though the final leg of the redirect is back to a same-origin page.

That's what the spec says to do and what Firefox does, but Blink handles CORS redirects separately from normal redirects, which are handled in net. These redirects go through DocumentThreadableLoader::DocumentThreadableLoader::makeCrossOriginAccessRequest(), which ends up recreating the Referer header from the document URL and referrer policy.

This will be a bit tricky to fix. We need to preserve the referrer when following redirects and apply the referrer policy to that, rather than starting fresh from the document's URL.

We also will need to properly set the referrer policy on the new request when following the redirect. Right now, in WebURLLoaderImpl::PopulateURLRequestForRedirect(), we set it to the original referrer policy, but that might have changed in a previous redirect, so we need to keep it up to date.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8aa9abc54f9bfedcce035846a2cc898d5b252957

commit 8aa9abc54f9bfedcce035846a2cc898d5b252957
Author: estark <estark@chromium.org>
Date: Tue Jul 19 16:13:32 2016

Preserve referrer across CORS-enabled redirects

When following a CORS-enabled redirect in DocumentThreadableLoader, use
the previous leg's referrer as the starting point instead of the
document's URL. This matches the Referrer Policy spec and ensures that
information is never added to the referrer when following a
redirect. (In other words, if the referrer is stripped in one leg of a
request, it will not be re-created in a subsequent leg.)

BUG= 628457 

Review-Url: https://codereview.chromium.org/2151693009
Cr-Commit-Position: refs/heads/master@{#406289}

[add] https://crrev.com/8aa9abc54f9bfedcce035846a2cc898d5b252957/third_party/WebKit/LayoutTests/http/tests/security/referrerPolicyHeader/referrer-policy-cross-origin-redirect.php
[add] https://crrev.com/8aa9abc54f9bfedcce035846a2cc898d5b252957/third_party/WebKit/LayoutTests/http/tests/security/referrerPolicyHeader/resources/redirect-to.php
[add] https://crrev.com/8aa9abc54f9bfedcce035846a2cc898d5b252957/third_party/WebKit/LayoutTests/http/tests/security/referrerPolicyHeader/resources/referrer-and-host.php
[modify] https://crrev.com/8aa9abc54f9bfedcce035846a2cc898d5b252957/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
[modify] https://crrev.com/8aa9abc54f9bfedcce035846a2cc898d5b252957/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h

Comment 2 by est...@chromium.org, Jul 19 2016

The commit in Comment 1 fixes the main problem, which is that the referrer is stripped and then recreated from the document's URL. We still need to properly set the referrer policy upon following a redirect. That's quite a bit trickier because the net stack doesn't directly expose the referrer policy but rather the net::URLRequest::ReferrerPolicy which doesn't map 1:1 to "real" referrer policies.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f5deae9b4797dfc2ce9ec685bc877c68982621b

commit 0f5deae9b4797dfc2ce9ec685bc877c68982621b
Author: estark <estark@chromium.org>
Date: Mon Oct 10 19:04:31 2016

Send net's ReferrerPolicy back to Blink while following redirects

//net might modify a request's referrer policy while following server
redirects that contain a Referrer-Policy header. Blink learns about the
redirects via the ResourceMsg_ReceivedRedirect IPC, and in some cases
(notably, CORS-enabled redirects), Blink chooses to tell //net not to
follow the redirect but instead kicks off a new request to follow the
redirect. (See DocumentThreadableLoader::redirectReceived().) In these
cases, Blink needs to know the referrer policy that //net assigned to
the request, based on the Referrer-Policy header that was received, so
that, for example, Blink can apply that referrer policy while following
a CORS-enabled redirect itself.

This CL changes WebURLLoaderImpl::PopulateURLRequestForRedirect() to
translate the request's net::URLRequest::ReferrerPolicy into a
WebReferrerPolicy that gets sent to Blink.

So that this translation can always be accurate, this CL also stops
using NEVER_CLEAR_REFERRER for multiple referrer policies ('origin',
'unsafe-url', and 'no-referrer') and instead always uses the
net::URLRequest::ReferrerPolicy corresponding to the actual referrer
policy for the request. That is: ORIGIN and NO_REFERRER are no longer
used only internally by URLRequest for processing Referrer-Policy
headers, but are now values that callers should use to convey a referrer
policy of 'origin' or 'no-referrer', so that the
net::URLRequest::ReferrerPolicy of a request can always be relied upon
to translate back to a WebReferrerPolicy.

BUG= 628457 

Review-Url: https://codereview.chromium.org/2393633006
Cr-Commit-Position: refs/heads/master@{#424203}

[modify] https://crrev.com/0f5deae9b4797dfc2ce9ec685bc877c68982621b/content/browser/loader/resource_dispatcher_host_impl.cc
[modify] https://crrev.com/0f5deae9b4797dfc2ce9ec685bc877c68982621b/content/child/web_url_loader_impl.cc
[modify] https://crrev.com/0f5deae9b4797dfc2ce9ec685bc877c68982621b/content/child/web_url_loader_impl.h
[modify] https://crrev.com/0f5deae9b4797dfc2ce9ec685bc877c68982621b/net/url_request/url_request.cc
[modify] https://crrev.com/0f5deae9b4797dfc2ce9ec685bc877c68982621b/net/url_request/url_request.h
[add] https://crrev.com/0f5deae9b4797dfc2ce9ec685bc877c68982621b/third_party/WebKit/LayoutTests/http/tests/security/referrerPolicyHeader/referrer-policy-header-on-cross-origin-redirect-response.https.html
[modify] https://crrev.com/0f5deae9b4797dfc2ce9ec685bc877c68982621b/third_party/WebKit/LayoutTests/http/tests/security/referrerPolicyHeader/resources/redirect-to.php
[modify] https://crrev.com/0f5deae9b4797dfc2ce9ec685bc877c68982621b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp

Comment 4 by est...@chromium.org, Oct 10 2016

Labels: -M-54 M-56
Status: Fixed (was: Assigned)

Sign in to add a comment