service workers should set referrer policy based on script headers

RESOLVED FIXED in Firefox 54

Status

()

Core
DOM: Service Workers
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: bkelly, Assigned: bkelly)

Tracking

(Blocks: 1 bug)

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(3 attachments, 6 obsolete attachments)

(Assignee)

Description

a year ago
We now set CSP based on service worker script headers, but we still don't set referrer policy.  We should fix that.
(Assignee)

Comment 1

a year ago
Related to this we don't apply the referrer policy header to other worker scripts correctly.  We currently do this on importScripts() in addition to the main worker script.  That appears to be incorrect.
(Assignee)

Comment 2

a year ago
Created attachment 8839262 [details] [diff] [review]
P2 Test that nested referrer policy from an importScripts() has no effect. r=baku

This tests the issue referenced in comment 1.
(Assignee)

Comment 3

a year ago
Created attachment 8839266 [details] [diff] [review]
P2 Test that nested referrer policy from an importScripts() has no effect. r=baku

Update wpt manifest.
Attachment #8839262 - Attachment is obsolete: true
(Assignee)

Comment 4

a year ago
Created attachment 8839274 [details] [diff] [review]
P3 Test referrer-policy in service workers. r=baku

Partial patch that converts some of the fetch referrer-policy tests to also run in service workers.
(Assignee)

Comment 5

a year ago
Created attachment 8839275 [details] [diff] [review]
P1 Set referrer policy in service workers. r=baku

Work-in-progress patch to fix the problem.  Does not address comment 1 yet.
(Assignee)

Comment 6

a year ago
Created attachment 8839296 [details] [diff] [review]
P1 Set referrer policy in service workers. r=baku

I believe this addresses all the problems in this bug.  I want to fix up the tests a bit more before flagging for review, though.
Attachment #8839275 - Attachment is obsolete: true
(Assignee)

Comment 7

a year ago
Created attachment 8839519 [details] [diff] [review]
P3 Test referrer-policy in service workers. r=baku
Attachment #8839274 - Attachment is obsolete: true
(Assignee)

Comment 8

a year ago
Created attachment 8839521 [details] [diff] [review]
P2 Test that nested referrer policy from an importScripts() has no effect. r=baku
Attachment #8839266 - Attachment is obsolete: true
(Assignee)

Comment 9

a year ago
Created attachment 8839524 [details] [diff] [review]
P1 Set referrer policy in service workers. r=baku
Attachment #8839296 - Attachment is obsolete: true
(Assignee)

Comment 11

a year ago
Comment on attachment 8839524 [details] [diff] [review]
P1 Set referrer policy in service workers. r=baku

This patch makes us apply the referrer-policy header to the WorkerPrivate for service workers coming out of CacheStorage in addition to the nsIChannel path.

It also only sets the referrer-policy if we are operating on the main script.  Previously we were incorrectly overriding the referrer-policy if an importScripts() had the header.
Attachment #8839524 - Flags: review?(amarchesini)
(Assignee)

Comment 12

a year ago
Comment on attachment 8839521 [details] [diff] [review]
P2 Test that nested referrer policy from an importScripts() has no effect. r=baku

This verifies that the nested importScripts() with a referrer-policy header does not affect the worker.
Attachment #8839521 - Flags: review?(amarchesini)
(Assignee)

Comment 13

a year ago
Comment on attachment 8839519 [details] [diff] [review]
P3 Test referrer-policy in service workers. r=baku

This expands the fetch tests to verify the referrer-policy works in service workers.

Note, I had to adjust some of the URLs in the test files to account for the tests being run from an https origin in service worker tests vs an http origin in the other tests.
Attachment #8839519 - Flags: review?(amarchesini)
Attachment #8839524 - Flags: review?(amarchesini) → review+
Attachment #8839521 - Flags: review?(amarchesini) → review+
Comment on attachment 8839519 [details] [diff] [review]
P3 Test referrer-policy in service workers. r=baku

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

::: testing/web-platform/meta/fetch/api/policies/referrer-origin-service-worker.https.html.ini
@@ +1,5 @@
> +[referrer-origin-service-worker.https.html]
> +  type: testharness
> +  [Cross-origin referrer is overridden by client origin]
> +    expected: FAIL
> +

Do we have a follow up for this? Can we write the bug ID in case?
Attachment #8839519 - Flags: review?(amarchesini) → review+
(Assignee)

Comment 15

a year ago
(In reply to Andrea Marchesini [:baku] from comment #14)
> Do we have a follow up for this? Can we write the bug ID in case?

Yes, I think its covered by both bug 1341223 and bug 1298823.
(Assignee)

Comment 16

a year ago
Created attachment 8840431 [details] [diff] [review]
P3 Test referrer-policy in service workers. r=baku
Attachment #8839519 - Attachment is obsolete: true
Attachment #8840431 - Flags: review+

Comment 18

a year ago
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a02f3229c14c
P1 Set referrer policy in service workers. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8ae4c3de328
P2 Test that nested referrer policy from an importScripts() has no effect. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/88206a320746
P3 Test referrer-policy in service workers. r=baku

Comment 19

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a02f3229c14c
https://hg.mozilla.org/mozilla-central/rev/b8ae4c3de328
https://hg.mozilla.org/mozilla-central/rev/88206a320746
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.