1845006 - Fix a few intertwined bugs with data URL parsing (double-parsing of the mime type, not fully serializing them, and doubling of charsets in content-type headers)

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[Thomas Wisniewski [:twisniewski]]

Basically, we already parse the mime type part of the URL here: https://searchfox.org/mozilla-central/source/netwerk/protocol/data/nsDataChannel.cpp#61
But SetContentType and SetContentCharset at the end of the function will also re-parse the result, using our older parser which isn't saving the parameters of the mime type: https://searchfox.org/mozilla-central/source/netwerk/protocol/data/nsDataChannel.cpp#102-103
We should just set mContentType and mContentCharset in the first line above.

Per spec we should also be serializing the full resulting mimetype here, not just getting its essence: https://searchfox.org/mozilla-central/source/netwerk/protocol/data/nsDataHandler.cpp#189

Finally, the fetch and XHR code doubles-up the charset needlessly in these spots, so we should tell it to only add the charset if it's not already present:

• https://searchfox.org/mozilla-central/source/dom/fetch/FetchDriver.cpp#1063-1069
• https://searchfox.org/mozilla-central/source/dom/xhr/XMLHttpRequestMainThread.cpp#1157-1165
• https://searchfox.org/mozilla-central/source/dom/xhr/XMLHttpRequestMainThread.cpp#1216-1222

Comment 1

[Thomas Wisniewski [:twisniewski]]

https://treeherder.mozilla.org/jobs?repo=try&revision=536704b4eb927bf7ca6d1b9f5e5186339fde57b9

Comment 2

[Thomas Wisniewski [:twisniewski]]

It turns out that too much Gecko code relies on whatever "content type" happens to mean right now, which is not "whatever is specified by the data url as per the spec", but seems to be more along the lines of the essence of the MimeType (more or less). As such it's too risky to change all of that, so I've taken a new approach:

• update our data url parsing code to follow the spec text more closely to pass these WPTs.
• have it put the full MimeType in a new instance var on the data url channel
• have XHR/fetch use that new value for content-type response headers on data urls.

New try run here: https://treeherder.mozilla.org/jobs?repo=try&revision=657429472db615aa68036814b8e8ead2a51409f5

Comment 3

[Thomas Wisniewski [:twisniewski]]

Attachment: Bug 1845006 - store the fully-serialized MimeType on data url channels so XHR and fetch may use it for content-type response headers, and clean up the data url parsing code to better match the spec. r?kershaw,sunil

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d98996f824c
store the fully-serialized MimeType on data url channels so XHR and fetch may use it for content-type response headers, and clean up the data url parsing code to better match the spec. r=kershaw,sunil,necko-reviewers

Comment 5

[Cristina Horotan [:chorotan]]

Backed out changeset 8d98996f824c (Bug 1845006) for causing build bustage at FetchDriver.cpp

Backout: https://hg.mozilla.org/integration/autoland/rev/23140ddbeb8d9d3b91d2e3111b0e640f249e1c02

Failure push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=8d98996f824c284a7c7ad1d4f2fa8872868139e9

Failure log: https://treeherder.mozilla.org/logviewer?job_id=424652254&repo=autoland&lineNumber=9467

Comment 6

[Thomas Wisniewski [:twisniewski]]

Ah, apologies. This is a quick fix I should have caught by rebuilding first after rebasing, then landing. Fix incoming later today.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f96e24bbd71c
store the fully-serialized MimeType on data url channels so XHR and fetch may use it for content-type response headers, and clean up the data url parsing code to better match the spec. r=kershaw,sunil,necko-reviewers

Comment 8

[Narcis Beleuzu [:NarcisB]]

Backed out for bustages on nsDataHandler.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/d490ff4c8be402a348becc31bc38bc4bf45193f0
Log link: https://treeherder.mozilla.org/logviewer?job_id=424688770&repo=autoland&lineNumber=31345

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0d9bc1bc0799
store the fully-serialized MimeType on data url channels so XHR and fetch may use it for content-type response headers, and clean up the data url parsing code to better match the spec. r=kershaw,sunil,necko-reviewers

Comment 10

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/0d9bc1bc0799