1264164 - Support Referrer Policy HTTP header

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Henry Chang [:hchang]]

We have implemented <meta> and per-element referrer policy. The next one to do is referrer policy carried by HTTP header.

Comment 1

[Ethan Tseng [:ethan]]

Hi Franziskus,

Do we have any plan on the timeline of implementing this feature?
I ask this because Henry and I are figuring out our job priorities.

Comment 2

[Franziskus Kiefer [:franziskus]]

When bug 1223838 landed this should be one of the last bugs to support all core features from Referrer Policy spec. If we could get both into 49, that would be great. But bug 1223838 is higher priority.

Comment 3

[Ethan Tseng [:ethan]]

(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #2)
> When bug 1223838 landed this should be one of the last bugs to support all
> core features from Referrer Policy spec. If we could get both into 49, that
> would be great. But bug 1223838 is higher priority.

Franziskus, thanks for your feedback.
Thomas is doing his best on bug 1223838. Hope we can get it landed in FF 49.

But for this bug, we have less than three weeks before FF 49 (June 6).
Henry is still busy working on SafeBrowsing bugs and didn't start this one yet.
I don't think we can get this bug done in such limited time frame.

Can we make the target of this bug as FF 50?

Comment 4

[Franziskus Kiefer [:franziskus]]

(In reply to Ethan Tseng [:ethan] from comment #3)
> Can we make the target of this bug as FF 50?

Sure! There's no rush here. No other browser is supporting this yet. 50 sounds like a good target to me.

Comment 5

[Thomas Nguyen (:tnguyen)]

Attachment: Add Referrer Policy HTTP header testcases

MozReview-Commit-ID: 2uMM7jXNVyF

Comment 6

[Thomas Nguyen (:tnguyen)]

Attachment: Enable Referrer Policy HTTP header web-platform-tests

MozReview-Commit-ID: zIi6j3Gzmg

Comment 7

[Thomas Nguyen (:tnguyen)]

Attachment: Support Referrer Policy HTTP header

MozReview-Commit-ID: 6KoTp9vuKCY

Comment 8

[Thomas Nguyen (:tnguyen)]

Attachment: Support Referrer Policy HTTP header

MozReview-Commit-ID: 6KoTp9vuKCY

Comment 9

[Thomas Nguyen (:tnguyen)]

Hi Josh,
Could you please take a look at this bug? 
The patches that I've added mostly implement the support of Referrer-Policy header to window(document)/worker context.
Also, there's a change in fetch's request referrer policy with this header.This bug still needs a little work on Bug 1264792 to complete fetch changes regarding to referrer policy, but I think we should resolve this first.

Comment 10

[Josh Matthews [:jdm]]

Comment on attachment 8771366 [details] [diff] [review]
Support Referrer Policy HTTP header

Review of attachment 8771366 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsDocument.cpp
@@ +3782,5 @@
>    // Referrer policy spec says to ignore any empty referrer policies.
> +  if ((aHeaderField == nsGkAtoms::referrer ||
> +       aHeaderField == nsGkAtoms::headerReferrerPolicy) &&
> +      !aData.IsEmpty()) {
> +     ReferrerPolicy policy = nsContentUtils::GetReferrerPolicyFromHeader(aData);

Using the new method for `<meta name="referrer" content="...">` tags means that comma-separated values will be allowed, which is contrary to the specification: https://html.spec.whatwg.org/multipage/semantics.html#meta-referrer

@@ +8736,5 @@
>        "x-frame-options",
>        // add more http headers if you need
>        // XXXbz don't add content-location support without reading bug
>        // 238654 and its dependencies/dups first.
> +      "referrer-policy",

Let's add this above the comment.

Comment 11

[Josh Matthews [:jdm]]

Comment on attachment 8771345 [details] [diff] [review]
Add Referrer Policy HTTP header testcases

Review of attachment 8771345 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/test/referrer_test_server.sjs
@@ +33,5 @@
> +  `;
> +}
> +
> +function handleRequest(request, response)
> +{

nit: { on the previous line.

@@ +38,5 @@
> +  var params = new URLSearchParams(request.queryString);
> +  var policy = params.get("Referrer-Policy");
> +  var type = params.get("TYPE");
> +  var action = params.get("ACTION");
> +  var csp = `default-src *`;

Why the `` here?

@@ +39,5 @@
> +  var policy = params.get("Referrer-Policy");
> +  var type = params.get("TYPE");
> +  var action = params.get("ACTION");
> +  var csp = `default-src *`;
> +  response.setHeader("Content-Security-Policy", csp, false);

Why not inline the header value?

::: dom/workers/test/referrer_worker.html
@@ +115,5 @@
> +  for (var type in testCases) {
> +    for (var policy in testCases[type]['Referrer-Policy']) {
> +      yield resetState();
> +      var searchParams = new URLSearchParams();
> +      searchParams.append("TYPE", escape(type));

Where's escape defined?

::: dom/workers/test/test_referrer_header_worker.html
@@ +7,5 @@
> +<head>
> +  <meta charset="utf-8">
> +  <title>Test the referrer of workers</title>
> +  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
> +  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" /></head>

nit: move the </head> to the next line.

::: testing/web-platform/meta/MANIFEST.json
@@ +15831,5 @@
>          "path": "fetch/api/policies/referrer-origin.html",
>          "url": "/fetch/api/policies/referrer-origin.html"
>        },
>        {
> +        "path": "fetch/api/policies/referrer-origin-when-cross-origin.html",

Did these new tests get created using `./mach web-platform-tests-create`? I wouldn't expect to see these changes in this part of the file if so.

::: testing/web-platform/tests/fetch/api/policies/referrer-origin-when-cross-origin.js
@@ +3,5 @@
> +  importScripts("../resources/utils.js");
> +}
> +
> +var referrerOrigin = "http://{{host}}:{{ports[http][0]}}/";
> +var fetchedUrl = "http://www1.{{host}}:{{ports[http][0]}}" + dirname(location.pathname) + RESOURCES_DIR + "inspect-headers.py?cors&headers=referer";

Let's use "http://{{host}}:{{ports[http][1]}}" here instead.

Comment 12

[Thomas Nguyen (:tnguyen)]

(In reply to Josh Matthews [:jdm] from comment #10)
> Comment on attachment 8771366 [details] [diff] [review]
> Support Referrer Policy HTTP header
> 
> Review of attachment 8771366 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/base/nsDocument.cpp
> @@ +3782,5 @@
> >    // Referrer policy spec says to ignore any empty referrer policies.
> > +  if ((aHeaderField == nsGkAtoms::referrer ||
> > +       aHeaderField == nsGkAtoms::headerReferrerPolicy) &&
> > +      !aData.IsEmpty()) {
> > +     ReferrerPolicy policy = nsContentUtils::GetReferrerPolicyFromHeader(aData);
> 
> Using the new method for `<meta name="referrer" content="...">` tags means
> that comma-separated values will be allowed, which is contrary to the
> specification:
> https://html.spec.whatwg.org/multipage/semantics.html#meta-referrer
> 
Oops, thanks for your prompt and great review.

Comment 13

[Thomas Nguyen (:tnguyen)]

Attachment: Add Referrer Policy HTTP header testcases

MozReview-Commit-ID: JMUr8DxAzVh

Comment 14

[Thomas Nguyen (:tnguyen)]

Attachment: Enable Referrer Policy HTTP header web-platform-tests

MozReview-Commit-ID: FKPCjpPjB41

Comment 15

[Thomas Nguyen (:tnguyen)]

Attachment: Support Referrer Policy HTTP header

MozReview-Commit-ID: JK4ZMyD0X2C

Comment 16

[Thomas Nguyen (:tnguyen)]

Attachment: Support Referrer Policy HTTP header

MozReview-Commit-ID: 1Acucn8CWVc

Comment 17

[Thomas Nguyen (:tnguyen)]

Attachment: Support Referrer Policy HTTP header

MozReview-Commit-ID: F2PpxwZsSWH

Comment 18

[Thomas Nguyen (:tnguyen)]

Try 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9821768ac38e

Comment 19

[Thomas Nguyen (:tnguyen)]

Attachment: Support Referrer Policy HTTP header

MozReview-Commit-ID: GANRqgAZQ5C

Comment 20

[Thomas Nguyen (:tnguyen)]

Attachment: Add Referrer Policy HTTP header testcases

MozReview-Commit-ID: JMUr8DxAzVh

Comment 21

[Thomas Nguyen (:tnguyen)]

Comment on attachment 8771825 [details] [diff] [review]
Add Referrer Policy HTTP header testcases

There're a new commits which is pushed to inbound/central, rebase the patches

Comment 22

[Thomas Nguyen (:tnguyen)]

Rebased to central/inbound branch today. :)

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2034653efbfd
Enable Referrer Policy HTTP header web-platform-tests. r=jdm
https://hg.mozilla.org/integration/mozilla-inbound/rev/81ad4d06462d
Support Referrer Policy HTTP header. r=jdm
https://hg.mozilla.org/integration/mozilla-inbound/rev/77fbb7ff85e5
Add Referrer Policy HTTP header testcases. r=jdm

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2034653efbfd
https://hg.mozilla.org/mozilla-central/rev/81ad4d06462d
https://hg.mozilla.org/mozilla-central/rev/77fbb7ff85e5

Comment 25

[Ethan Tseng [:ethan]]

Thomas, thanks for getting this done!
It's a big step for us!

Comment 26

[Thomas Nguyen (:tnguyen)]

Attachment: Bug 1264164 - Update request'referrer policy when redirect.

Review commit: https://reviewboard.mozilla.org/r/75490/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/75490/

Comment 27

[Thomas Nguyen (:tnguyen)]

Attachment: Bug 1264164 - Add Fetch redirect referrer policy test case.

Review commit: https://reviewboard.mozilla.org/r/75492/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/75492/

Comment 28

[Thomas Nguyen (:tnguyen)]

Comment on attachment 8786588 [details]
Bug 1264164 - Update request'referrer policy when redirect.

https://reviewboard.mozilla.org/r/75488/#review73456

Wrong bug number

Comment 29

[Florian Scholz (Open Web Docs)]

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referrer-Policy
https://developer.mozilla.org/en-US/Firefox/Releases/50#HTTP