Fetch: content-length header is being added to the safe-list
Categories
(Core :: DOM: Networking, defect, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox87 | --- | fixed |
People
(Reporter: shacharz, Assigned: fli+bugzilla)
References
(Blocks 2 open bugs)
Details
(Keywords: dev-doc-complete, Whiteboard: [necko-triaged], [wptsync upstream])
Attachments
(2 files)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36 Steps to reproduce: 1. create a CORS fetch request to a url that has content-length in its response and doesn't add content-length to access-control-expose-headers 2. content-length should be accessible in the response's headers Actual results: content-length is not exposed Expected results: When doing a CORS fetch request if content-length exists in the http response it should be exposed on the response headers even if it's not added to the access-control-expose-headers. The spec is under change: https://github.com/whatwg/fetch/pull/626 https://github.com/w3c/web-platform-tests/pull/10930
|
||
Updated•6 years ago
|
|
||
Updated•6 years ago
|
Hey any update here? Edge fixed the issue - https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/17440391/ Webkit as well - https://bugs.webkit.org/show_bug.cgi?id=185473
|
||
Updated•5 years ago
|
Who would be the most suitable person to review the changes? This is my first time here, so I'm not familiar with the process of requesting a reviewer.
|
||
Comment 5•4 years ago
|
||
Hey Forrest (and Shachar), apologies for the lack of follow-up here. Junior can most likely take a look at your patch.
|
||
Comment 6•4 years ago
|
||
Hello Forrest, do you want to continue the process?
I can do the review. Just set the reviewer to JuniorHsu in phabricator.
Sorry for the extended delay. I'm pretty sure I accidentally deleted the notification emails a while back when mass-deleting old emails, and I'm just revisiting this.
Yes, I'm interested in moving forward with this patch. However, it appears that Junior's account is disabled in Phabricator. Is there anyone else I can set as the reviewer?
|
||
Updated•4 years ago
|
|
||
Comment 8•4 years ago
|
||
I think Andrea [baku] might be able to review this based on some blame info for InternalHeaders.cpp. I added them as a reviewer.
|
||
Updated•4 years ago
|
Hi all, what's the status on this issue? Is this content-length now added as a safelisted header now?
|
|
||
Comment 10•3 years ago
|
||
ehsueh, thanks for the reminder. Valentin, can you ensure https://phabricator.services.mozilla.com/D58492 lands? Looks like all is in order there.
|
|
||
Updated•3 years ago
|
|
||
Updated•3 years ago
|
|
||
Comment 11•3 years ago
|
||
Pushed by valentin.gosu@gmail.com: https://hg.mozilla.org/integration/autoland/rev/aefbe65483ef Add content-length as a CORS-safelisted response header. r=valentin,baku
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/27481 for changes under testing/web-platform/tests
|
||
Comment 13•3 years ago
|
||
Backed out for causing failures cors-filtering.sub.any.worker.html.
Backout link: https://hg.mozilla.org/integration/autoland/rev/55cf2cda9a1002e345e5f5821701fb314fa2aab1
Failure log: https://treeherder.mozilla.org/logviewer?job_id=328841702&repo=autoland&lineNumber=2485
|
|
||
Comment 14•3 years ago
•
|
||
It seems you just need to remove this file:
https://searchfox.org/mozilla-central/rev/927e525f481a93a8f63d27a78ae6201e42b1b1fb/testing/web-platform/meta/fetch/api/cors/cors-filtering.sub.any.js.ini
Upstream PR was closed without merging
|
Assignee | |
Comment 16•3 years ago
|
||
I've updated that test-case to expect passing results.
|
|
||
Comment 17•3 years ago
|
||
Depends on D58492
|
|
||
Comment 18•3 years ago
|
||
Pushed by valentin.gosu@gmail.com: https://hg.mozilla.org/integration/autoland/rev/4615f67be1a8 Add content-length as a CORS-safelisted response header. r=valentin,baku https://hg.mozilla.org/integration/autoland/rev/3b0711521076 Remove cors-filtering.sub.any.js.ini a=test-only
|
|
Assignee | |
Comment 19•3 years ago
|
||
Oh thanks! I pushed the change and didn't notice that it was already closed after you made the change. You can ignore the recent change on phabricator.
|
||
Comment 20•3 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/4615f67be1a8
https://hg.mozilla.org/mozilla-central/rev/3b0711521076
Upstream PR merged by moz-wptsync-bot
|
||
Comment 22•3 years ago
|
||
FF87 docs for this are being done as part of https://github.com/mdn/content/pull/2766. Still waiting on discussion on BCD. Note also a little scope creep in docs for this one because I found the existing descriptions for the CORS response header safelist a bit confusing.
|
||
Comment 23•3 years ago
|
||
Setting this one to DDC; the BCD work is very nearly there.
Description
•