Closed Bug 1340654 Opened 3 years ago Closed 3 years ago

service workers should set referrer policy based on script headers

Categories

(Core :: DOM: Service Workers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 6 obsolete files)

We now set CSP based on service worker script headers, but we still don't set referrer policy.  We should fix that.
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.
Update wpt manifest.
Attachment #8839262 - Attachment is obsolete: true
Partial patch that converts some of the fetch referrer-policy tests to also run in service workers.
Work-in-progress patch to fix the problem.  Does not address comment 1 yet.
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
Attachment #8839274 - Attachment is obsolete: true
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)
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)
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+
(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.
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
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
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.