Bug 175069 - Improve our support for referrer policies
Summary: Improve our support for referrer policies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: https://www.w3.org/TR/referrer-policy...
Keywords: InRadar
: 154588 170061 (view as bug list)
Depends on: 175089
Blocks: 147885 186037
  Show dependency treegraph
 
Reported: 2017-08-02 08:39 PDT by Chris Dumez
Modified: 2018-05-29 04:23 PDT (History)
15 users (show)

See Also:


Attachments
WIP Patch (44.50 KB, patch)
2017-08-02 13:10 PDT, Chris Dumez
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-elcapitan (1.02 MB, application/zip)
2017-08-02 14:11 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (1.44 MB, application/zip)
2017-08-02 14:59 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-elcapitan (1.86 MB, application/zip)
2017-08-02 15:34 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (1.24 MB, application/zip)
2017-08-02 16:05 PDT, Build Bot
no flags Details
WIP Patch (53.38 KB, patch)
2017-08-02 16:23 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-elcapitan (1.11 MB, application/zip)
2017-08-02 17:22 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-elcapitan (1.88 MB, application/zip)
2017-08-02 19:31 PDT, Build Bot
no flags Details
Patch (66.29 KB, patch)
2017-08-02 21:21 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (66.10 KB, patch)
2017-08-03 09:39 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2017-08-02 08:39:57 PDT
Improve our support for referrer policies:
- https://www.w3.org/TR/referrer-policy/#referrer-policies
Comment 1 Radar WebKit Bug Importer 2017-08-02 08:40:23 PDT
<rdar://problem/33677313>
Comment 2 Chris Dumez 2017-08-02 13:10:14 PDT
Created attachment 316986 [details]
WIP Patch
Comment 3 Build Bot 2017-08-02 14:11:28 PDT
Comment on attachment 316986 [details]
WIP Patch

Attachment 316986 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4241511

New failing tests:
http/tests/referrer-policy/origin-when-cross-origin/cross-origin-http.https.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-referrer.html
http/tests/referrer-policy/same-origin/cross-origin-http.https.html
http/tests/referrer-policy/strict-origin-when-cross-origin/cross-origin-http.https.html
http/tests/referrer-policy/strict-origin/cross-origin-http.https.html
imported/w3c/web-platform-tests/fetch/api/request/request-init-001.sub.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-referrer-worker.html
Comment 4 Build Bot 2017-08-02 14:11:29 PDT
Created attachment 316992 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 5 Build Bot 2017-08-02 14:59:24 PDT
Comment on attachment 316986 [details]
WIP Patch

Attachment 316986 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4241694

New failing tests:
imported/w3c/web-platform-tests/fetch/api/request/request-init-001.sub.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-referrer.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-referrer-worker.html
Comment 6 Build Bot 2017-08-02 14:59:25 PDT
Created attachment 317009 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 7 Build Bot 2017-08-02 15:34:45 PDT
Comment on attachment 316986 [details]
WIP Patch

Attachment 316986 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4241870

New failing tests:
http/tests/referrer-policy/origin-when-cross-origin/cross-origin-http.https.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-referrer.html
http/tests/referrer-policy/same-origin/cross-origin-http.https.html
http/tests/referrer-policy/strict-origin-when-cross-origin/cross-origin-http.https.html
http/tests/referrer-policy/strict-origin/cross-origin-http.https.html
imported/w3c/web-platform-tests/fetch/api/request/request-init-001.sub.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-referrer-worker.html
Comment 8 Build Bot 2017-08-02 15:34:47 PDT
Created attachment 317017 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 9 Build Bot 2017-08-02 16:05:02 PDT
Comment on attachment 316986 [details]
WIP Patch

Attachment 316986 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4242103

New failing tests:
imported/w3c/web-platform-tests/fetch/api/request/request-init-001.sub.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-referrer.html
imported/w3c/web-platform-tests/fetch/api/redirect/redirect-referrer-worker.html
Comment 10 Build Bot 2017-08-02 16:05:04 PDT
Created attachment 317022 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 11 Chris Dumez 2017-08-02 16:23:34 PDT
Created attachment 317027 [details]
WIP Patch
Comment 12 Build Bot 2017-08-02 17:22:33 PDT
Comment on attachment 317027 [details]
WIP Patch

Attachment 317027 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4242665

New failing tests:
http/tests/referrer-policy/origin-when-cross-origin/cross-origin-http.https.html
http/tests/referrer-policy/same-origin/cross-origin-http.https.html
http/tests/referrer-policy/strict-origin/cross-origin-http.https.html
http/tests/referrer-policy/strict-origin-when-cross-origin/cross-origin-http.https.html
Comment 13 Build Bot 2017-08-02 17:22:35 PDT
Created attachment 317040 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 14 Build Bot 2017-08-02 19:31:54 PDT
Comment on attachment 317027 [details]
WIP Patch

Attachment 317027 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4243337

New failing tests:
http/tests/referrer-policy/origin-when-cross-origin/cross-origin-http.https.html
http/tests/referrer-policy/same-origin/cross-origin-http.https.html
http/tests/referrer-policy/strict-origin/cross-origin-http.https.html
http/tests/referrer-policy/strict-origin-when-cross-origin/cross-origin-http.https.html
Comment 15 Build Bot 2017-08-02 19:31:55 PDT
Created attachment 317067 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 16 Chris Dumez 2017-08-02 21:21:01 PDT
Created attachment 317087 [details]
Patch
Comment 17 Darin Adler 2017-08-03 09:35:56 PDT
Comment on attachment 317087 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=317087&action=review

> Source/WebCore/dom/Document.cpp:3381
> +    // "never" / "default" / "always" are legacy keywords that we will support. They were defined in:
> +    // https://www.w3.org/TR/2014/WD-referrer-policy-20140807/#referrer-policy-delivery-meta

We plan to support them forever?

> Source/WebCore/page/SecurityPolicy.cpp:92
> +        RELEASE_ASSERT_NOT_REACHED();

Does this really need to be RELEASE_ASSERT_NOT_REACHED? What’s the best guideline for this sort of thing? Should we get rid of ASSERT_NOT_REACHED? Why not just fall into NoReferrer after ASSERT_NOT_REACHED in this case in production builds? I think of RELEASE_ASSERT as something we use only in unusual circumstances.
Comment 18 Chris Dumez 2017-08-03 09:37:26 PDT
Comment on attachment 317087 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=317087&action=review

>> Source/WebCore/page/SecurityPolicy.cpp:92
>> +        RELEASE_ASSERT_NOT_REACHED();
> 
> Does this really need to be RELEASE_ASSERT_NOT_REACHED? What’s the best guideline for this sort of thing? Should we get rid of ASSERT_NOT_REACHED? Why not just fall into NoReferrer after ASSERT_NOT_REACHED in this case in production builds? I think of RELEASE_ASSERT as something we use only in unusual circumstances.

This was meant to be an ASSERT_NOT_REACHED().
Comment 19 Chris Dumez 2017-08-03 09:39:16 PDT
Created attachment 317126 [details]
Patch
Comment 20 Chris Dumez 2017-08-03 09:43:20 PDT
(In reply to Darin Adler from comment #17)
> Comment on attachment 317087 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=317087&action=review
> 
> > Source/WebCore/dom/Document.cpp:3381
> > +    // "never" / "default" / "always" are legacy keywords that we will support. They were defined in:
> > +    // https://www.w3.org/TR/2014/WD-referrer-policy-20140807/#referrer-policy-delivery-meta
> 
> We plan to support them forever?

I can look into this separately. I did not want to drop them in this patch.
We should test other browsers to see what they support as well. I know they support the new ones but I did not check if they still support the legacy ones.
Comment 21 WebKit Commit Bot 2017-08-03 10:19:48 PDT
Comment on attachment 317126 [details]
Patch

Clearing flags on attachment: 317126

Committed r220208: <http://trac.webkit.org/changeset/220208>
Comment 22 WebKit Commit Bot 2017-08-03 10:19:50 PDT
All reviewed patches have been landed.  Closing bug.
Comment 23 Daniel Bates 2017-08-31 17:15:54 PDT
*** Bug 170061 has been marked as a duplicate of this bug. ***
Comment 24 Daniel Bates 2017-08-31 17:17:10 PDT
*** Bug 154588 has been marked as a duplicate of this bug. ***
Comment 25 Derk-Jan Hartman 2018-05-24 08:59:44 PDT
I can confirm that origin-when-cross-origin now seems to be working for Wikipedia/Wikimedia for clients of Safari 11.1 and newer.
See also graph: https://phab.wmfusercontent.org/file/data/yijasv2ejcxgdiuoo2eq/PHID-FILE-z5vp7l7fbgewcbisj5qv/Internal_refferers_safari.png

I do note, that we currently have an error message in the console: "[Error] Failed to set referrer policy: The value 'origin-when-crossorigin' is not one of 'no-referrer', 'no-referrer-when-downgrade', 'same-origin', 'origin', 'strict-origin', 'origin-when-cross-origin', 'strict-origin-when-cross-origin' or 'unsafe-url'. Defaulting to 'no-referrer'.". I determined this is because we also specify the old misspelled "origin-when-crossorigin" after it implemented a fallback chain of referrers in February of this year. 
https://phabricator.wikimedia.org/T180921

We emit in the following order:
<meta name="referrer" content="origin">
<meta name="referrer" content="origin-when-crossorigin">
<meta name="referrer" content="origin-when-cross-origin">

Screenshot of error: https://phabricator.wikimedia.org/F18377524


I believe this error also hides that the fallback implementation isn't working. It shouldn't fallback to no-referrer, but to origin, if I read https://w3c.github.io/webappsec-referrer-policy/#unknown-policy-values correctly.
This might be come problematic if we later want to switch to newer values in the future. Shall I file a separate ticket for this problem ?