Open Bug 1697421 Opened 3 years ago Updated 1 month ago

XMLHttpRequest and fetch() don't combine Content-Length/Content-Type headers

Categories

(Core :: DOM: Networking, defect, P2)

Product:
Core
Component:
DOM: Networking
Type:
defect

Tracking

()

Status:
ASSIGNED
Tracking Flags:
Tracking Status
firefox107 --- affected
firefox108 --- affected

People

(Reporter: annevk, Assigned: smayya, Mentored)

References

(Blocks 4 open bugs)

Details

(Whiteboard: [necko-triaged])

Attachments

(5 files, 1 obsolete file)

Bug 1697421 - Make XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=#necko
48 bytes, text/x-phabricator-request
Details | Review
Bug 1697421 - store original response headers for 304 and 206 response. r=#necko
48 bytes, text/x-phabricator-request
Details | Review
Bug 1697421 - improve parsing of content-type headers for fetch r=#necko
48 bytes, text/x-phabricator-request
Details | Review
Bug 1697421 - update error logging for mime type parsing failures in web assembly. r=#necko
48 bytes, text/x-phabricator-request
Details | Review
Bug 1697421 - adjust some text expectations to account for wpt updates not applied to v107. r=dmeehan
48 bytes, text/x-phabricator-request
Details | Review

In tests added in https://github.com/web-platform-tests/wpt/pull/27837, rather than returning two Content-Length headers as "6, 6", I get "6", which suggests a header got dropped. Equivalent thing happens for Content-Type.

For XMLHttpRequest (didn't look into fetch()) this is because of this code:

https://searchfox.org/mozilla-central/source/dom/xhr/XMLHttpRequestMainThread.cpp#1194-1216

It seems that was added to fake response headers for blob URLs, but it ends up violating the requirements of the specification.

Blocks: xhr, fetch
Severity: -- → S4
Priority: -- → P2
Whiteboard: [necko-triaged]

This is not caused by blob URLs. This line should guard against that.
The fix should be calling GetOriginalResponseHeader instead of GetResponseHeader to catch multiple headers.

Mentor: valentin.gosu
Priority: P2 → P3
Assignee: nobody → smayya
Status: NEW → ASSIGNED
Attachment #9290246 - Attachment description: Bug 1697421 - modify GetResponseHeader APIs for XMLHttpRequest, fetch to support multiple content-length and content-type headers. r=#necko → Bug 1697421 - modify GetResponseHeader/GetAllResponseHeaders APIs for XMLHttpRequest to support multiple content-length and content-type headers. r=#necko
Attachment #9290246 - Attachment description: Bug 1697421 - modify GetResponseHeader/GetAllResponseHeaders APIs for XMLHttpRequest to support multiple content-length and content-type headers. r=#necko → Bug 1697421 - modify GetResponseHeader/GetAllResponseHeaders APIs for XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=#necko
Attachment #9290246 - Attachment description: Bug 1697421 - modify GetResponseHeader/GetAllResponseHeaders APIs for XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=#necko → WIP: Bug 1697421 - modify GetResponseHeader/GetAllResponseHeaders APIs for XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=#necko

Depends on D154864

Attachment #9293646 - Attachment is obsolete: true
Attachment #9290246 - Attachment description: WIP: Bug 1697421 - modify GetResponseHeader/GetAllResponseHeaders APIs for XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=#necko → Bug 1697421 - modify GetResponseHeader/GetAllResponseHeaders APIs for XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=#necko
Attachment #9293649 - Attachment description: Bug 1697421 - update handling of fetch response headers. r=#necko → WIP: Bug 1697421 - update handling of fetch response headers. r=#necko
Attachment #9294238 - Attachment description: Bug 1697421 - improve parsing of content-type headers for fetch r=#necko → WIP: Bug 1697421 - improve parsing of content-type headers for fetch r=#necko
Attachment #9293649 - Attachment description: WIP: Bug 1697421 - update handling of fetch response headers. r=#necko → Bug 1697421 - update handling of fetch response headers. r=#necko
Attachment #9294238 - Attachment description: WIP: Bug 1697421 - improve parsing of content-type headers for fetch r=#necko → Bug 1697421 - improve parsing of content-type headers for fetch r=#necko
Attachment #9290246 - Attachment description: Bug 1697421 - modify GetResponseHeader/GetAllResponseHeaders APIs for XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=#necko → Bug 1697421 - Make XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=#necko
Attachment #9293649 - Attachment description: Bug 1697421 - update handling of fetch response headers. r=#necko → Bug 1697421 - store original response headers for 304 and 206 response. r=#necko
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/fe6151585dec
Make XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/cda8626f9ccc
store original response headers for 304 and 206 response. r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/65d4f5822383
improve parsing of content-type headers for fetch  r=necko-reviewers,valentin

Backed out for causing mochitest failures on test_webassembly_compile.html.

Push with failures

Failure log

Backout link

[task 2022-09-15T21:17:00.530Z] 21:17:00     INFO - TEST-PASS | dom/promise/tests/test_webassembly_compile.html | correct MIME type error message 
[task 2022-09-15T21:17:00.530Z] 21:17:00     INFO - Buffered messages finished
[task 2022-09-15T21:17:00.532Z] 21:17:00     INFO - TEST-UNEXPECTED-FAIL | dom/promise/tests/test_webassembly_compile.html | correct MIME type error message - got "WebAssembly: Response has unsupported MIME type '' expected 'application/wasm'", expected "WebAssembly: Response has unsupported MIME type 'application/js;application/wasm' expected 'application/wasm'"
[task 2022-09-15T21:17:00.532Z] 21:17:00     INFO -     SimpleTest.is@SimpleTest/SimpleTest.js:497:14
[task 2022-09-15T21:17:00.532Z] 21:17:00     INFO -     compileStreamingBadMime/<@dom/promise/tests/test_webassembly_compile.html:172:11
[task 2022-09-15T21:17:00.533Z] 21:17:00     INFO - Not taking screenshot here: see the one that was previously logged
[task 2022-09-15T21:17:00.535Z] 21:17:00     INFO - TEST-UNEXPECTED-FAIL | dom/promise/tests/test_webassembly_compile.html | correct MIME type error message - got "WebAssembly: Response has unsupported MIME type '' expected 'application/wasm'", expected "WebAssembly: Response has unsupported MIME type 'application/wasm;application/js' expected 'application/wasm'"
[task 2022-09-15T21:17:00.535Z] 21:17:00     INFO -     SimpleTest.is@SimpleTest/SimpleTest.js:497:14
[task 2022-09-15T21:17:00.535Z] 21:17:00     INFO -     compileStreamingBadMime/<@dom/promise/tests/test_webassembly_compile.html:172:11
[task 2022-09-15T21:17:00.536Z] 21:17:00     INFO - Not taking screenshot here: see the one that was previously logged
[task 2022-09-15T21:17:00.537Z] 21:17:00     INFO - TEST-UNEXPECTED-FAIL | dom/promise/tests/test_webassembly_compile.html | correct MIME type error message - got "WebAssembly: Response has unsupported MIME type '' expected 'application/wasm'", expected "WebAssembly: Response has unsupported MIME type 'application/wasm;' expected 'application/wasm'"
[task 2022-09-15T21:17:00.538Z] 21:17:00     INFO -     SimpleTest.is@SimpleTest/SimpleTest.js:497:14
[task 2022-09-15T21:17:00.538Z] 21:17:00     INFO -     compileStreamingBadMime/<@dom/promise/tests/test_webassembly_compile.html:172:11
[task 2022-09-15T21:17:00.539Z] 21:17:00     INFO - TEST-PASS | dom/promise/tests/test_webassembly_compile.html | correct MIME type error message
Flags: needinfo?(smayya)
Flags: needinfo?(valentin.gosu)
Flags: needinfo?(smayya)
Attachment #9290246 - Attachment description: Bug 1697421 - Make XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=#necko → WIP: Bug 1697421 - Make XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=#necko
Attachment #9293649 - Attachment description: Bug 1697421 - store original response headers for 304 and 206 response. r=#necko → WIP: Bug 1697421 - store original response headers for 304 and 206 response. r=#necko
Attachment #9294238 - Attachment description: Bug 1697421 - improve parsing of content-type headers for fetch r=#necko → WIP: Bug 1697421 - improve parsing of content-type headers for fetch r=#necko
Attachment #9290246 - Attachment description: WIP: Bug 1697421 - Make XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=#necko → Bug 1697421 - Make XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=#necko
Attachment #9293649 - Attachment description: WIP: Bug 1697421 - store original response headers for 304 and 206 response. r=#necko → Bug 1697421 - store original response headers for 304 and 206 response. r=#necko
Attachment #9294238 - Attachment description: WIP: Bug 1697421 - improve parsing of content-type headers for fetch r=#necko → Bug 1697421 - improve parsing of content-type headers for fetch r=#necko
Attachment #9295224 - Attachment description: WIP: Bug 1697421 - update test-case expectations for WebAssembly.compile Test → Bug 1697421 - update error logging for mime type parsing failures in web assembly. r=#necko
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5979ac92fa24
Make XMLHttpRequest/fetch to support multiple content-length and content-type headers. r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/ca500b60941e
store original response headers for 304 and 206 response. r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/b1c8d75d49ef
improve parsing of content-type headers for fetch  r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/0ec0d4234b77
update error logging for mime type parsing failures in web assembly. r=necko-reviewers,valentin,tschuster
Regressions: 1792190

Valentin, at this moment there is an issue with the backout of this bug.
I`m trying to fix the issue and will let you know when the backout is done

Backed out for e.g. breaking Tampermonkey userscripts (bug 1798149).

Backout link: https://hg.mozilla.org/integration/autoland/rev/15acd3b1e975fe6e10d37b3ef88a010ed3504a96

Status: RESOLVED → REOPENED
Flags: needinfo?(smayya)
Resolution: FIXED → ---
Target Milestone: 107 Branch → ---
Flags: needinfo?(smayya)

Sunil, :valentin, I just tried this patch-stack on a fresh moz-central branch, and wasn't able to reproduce bug 1798149. Maybe we should try landing it again?

Flags: needinfo?(valentin.gosu)
Flags: needinfo?(smayya)

Hello Thomas,
Thanks for checking. I had actually pushed it to try last week, we have the following regression ->
We have new issue here with try.
The problem here is that the extension code is modifying the response headers.
However, it does not modify the original response header from the network resulting in inconsistent response headers being stored internally.
I will be looking into it next week once I have some cycles.

Flags: needinfo?(smayya)
Duplicate of this bug: 1491010

The severity field for this bug is set to S4. However, the following bug duplicate has higher severity:

:smayya, could you consider increasing the severity of this bug to S3?

For more information, please visit BugBot documentation.

Flags: needinfo?(smayya)
Severity: S4 → S3
Flags: needinfo?(smayya)
Priority: P3 → P2
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: