Bug 175069 – Improve our support for referrer policies

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Improve our support for referrer policies:
- https://www.w3.org/TR/referrer-policy/#referrer-policies

[Radar WebKit Bug Importer]

<rdar://problem/33677313>

[Chris Dumez]

Created attachment 316986 [details]
WIP Patch

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Chris Dumez]

Created attachment 317027 [details]
WIP Patch

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Chris Dumez]

Created attachment 317087 [details]
Patch

[Darin Adler]

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.

[Chris Dumez]

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().

[Chris Dumez]

Created attachment 317126 [details]
Patch

[Chris Dumez]

(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.

[WebKit Commit Bot]

Comment on attachment 317126 [details]
Patch

Clearing flags on attachment: 317126

Committed r220208: <http://trac.webkit.org/changeset/220208>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Daniel Bates]

*** Bug 170061 has been marked as a duplicate of this bug. ***

[Daniel Bates]

*** Bug 154588 has been marked as a duplicate of this bug. ***

[Derk-Jan Hartman]

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 ?