Failure in xhr/access-control-basic-cors-safelisted-request-headers.htm
Categories
(Core :: DOM: Networking, enhancement, P1)
Tracking
()
People
(Reporter: twisniewski, Assigned: twisniewski)
References
(Blocks 1 open bug)
Details
(Whiteboard: [necko-triaged][wptsync upstream])
Attachments
(1 file)
|
(deleted),
text/x-phabricator-request
|
Details |
This WPT is failing because our XHR code's list of CORS-safelisted request headers is out of sync with the spec: https://fetch.spec.whatwg.org/#cors-safelisted-request-header That is, we're incorrectly allowing last-event-id, while not allowing any of: dpr, downlink, save-data, viewport-width, width
|
Assignee | |
Comment 1•6 years ago
|
||
Our fetch code is also out-of-sync with the spec, and isn't safelisting dpr, downlink, save-data, viewport-width or width.
|
||
Updated•6 years ago
|
|
|
Assignee | |
Comment 2•6 years ago
|
||
I have a patch ready on try, seems fine so far: https://treeherder.mozilla.org/#/jobs?repo=try&revision=34329895722a647e399d4cd8ddf48773da18ec35 Properly passing the related WPTs required implementing the bits of the spec which validate not just the header names, but also their values (which included finding the relevant Client-Hints specs' definitions of what "valid values" actually were for those headers).
|
|
Assignee | |
Comment 3•6 years ago
|
||
update our cors/no-cors header safelisting to match the Fetch spec
|
||
Comment 4•6 years ago
|
||
(feel free to tweak the importance, but since this is already assigned and has a patch, I figured it might be a P1)
|
||
Updated•6 years ago
|
|
||
Comment 5•6 years ago
|
||
Jonathan is doing some work on this over in Bug 1483815. Let's just get Bug 1483815 landed and then use this bug to do the remaining bits the other bug is not addressing.
|
|
Assignee | |
Comment 6•5 years ago
|
||
Alright, I've updated the patch on phabricator to add the bits that Jonathan's work didn't add in bug 1483815, and also address review feedback from the last round. A fresh try run seems fine to me: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a648f2f3cc58e554abfd534f540e29b2675b8326
For now the new patch only addresses XHRs, not Fetch. If it lands and doesn't show any signs of problems, I'll file a follow-up bug to update Fetch accordingly.
|
|
Assignee | |
Comment 7•5 years ago
|
||
It appears that the WPT here is out-of-sync with the Fetch spec, and in fact the intent is to remove the Client Hints headers from the safelisted request headers. I'll wait to confirm all of that before updating my patch.
|
||
Comment 8•5 years ago
|
||
For the Client Hints thing Thomas, see https://github.com/whatwg/fetch/issues/867 and https://github.com/whatwg/fetch/pull/773. It's not entirely clear what the approach will end up being and I'm also not sure to what extent we're committed to any particular outcome.
|
|
Assignee | |
Comment 9•5 years ago
|
||
I see. Thanks, Anne. It still appears that (for now) the spec will drop the Client Hints bits entirely, until a proper outcome is made. So it seems that this bug/patch should just focus on aligning Firefox with the the non-CH bits for now.
|
||
Comment 10•5 years ago
|
||
Pushed by twisniewski@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/aaedb3306c2e update our cors/no-cors header safelisting to match the Fetch spec; r=hsivonen,ckerschb
|
||
Comment 11•5 years ago
|
||
| bugherder | ||
|
|
||
Comment 12•5 years ago
|
||
Backout by csabou@mozilla.com: https://hg.mozilla.org/mozilla-central/rev/ce51e7ec806c Backed out changeset aaedb3306c2e in attempt to fix the wpt failures on fetch.any.serviceworker.html. a=merge
|
|
||
Comment 13•5 years ago
|
||
Pushed by csabou@mozilla.com: https://hg.mozilla.org/mozilla-central/rev/3ebf6f2a8138 update our cors/no-cors header safelisting to match the Fetch spec; r=hsivonen,ckerschb a=reland
|
||
Updated•5 years ago
|
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16663 for changes under testing/web-platform/tests
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/17071 for changes under testing/web-platform/tests
Upstream PR was closed without merging
Description
•