See https://github.com/whatwg/xhr/pull/176 and https://github.com/w3c/web-platform-tests/pull/8422. In particular, XMLHttpRequest Content-Type manipulation is now to be done in terms of the MIME type parser defined in MIME Sniffing.
https://github.com/whatwg/xhr/pull/224 and https://github.com/web-platform-tests/wpt/pull/13779 have minor tweaks to this to make this more web-compatible.
Test is xhr/send-content-type-charset.htm.
Quite a few failures on imported/w3c/web-platform-tests/xhr/send-content-type-charset.htm: FAIL header with invalid MIME type is not changed assert_equals: expected "text; charset=ascii" but got "text; charset=UTF-8" PASS header with invalid MIME type (empty string) is not changed PASS known charset but bogus header - missing MIME type PASS bogus charset and bogus header - missing MIME type FAIL If charset= param is UTF-8 (case-insensitive), it should not be changed assert_equals: expected "text/plain;charset=utf-8" but got "text/plain;charset=UTF-8" PASS If no charset= param is given, implementation should not add one - unknown MIME PASS If no charset= param is given, implementation should not add one - known MIME PASS If no charset= param is given, implementation should not add one - known MIME, unknown param, two spaces FAIL charset given but wrong, fix it (unknown MIME, bogus charset) assert_equals: expected "text/x-thepiano;charset=UTF-8" but got "text/x-thepiano;charset= UTF-8" FAIL If charset= param is UTF-8 (case-insensitive), it should not be changed (bogus charset) assert_equals: expected "text/plain;charset=utf-8;charset=waddup" but got "text/plain;charset=UTF-8;charset=UTF-8" PASS charset given but wrong, fix it (known MIME, actual charset) FAIL Multiple non-UTF-8 charset parameters deduplicate, bogus parameter dropped assert_equals: expected "text/x-pink-unicorn;charset=UTF-8" but got "text/x-pink-unicorn; charset=UTF-8; charset=UTF-8; notrelated; charset=UTF-8" PASS No content type set, give MIME and charset FAIL charset with space that is UTF-8 does not change assert_equals: expected "text/plain;charset= utf-8" but got "text/plain;charset= UTF-8" FAIL charset in double quotes that is UTF-8 does not change assert_equals: expected "text/plain;charset=\"utf-8\"" but got "text/plain;charset=\"UTF-8\"" FAIL charset in double quotes with space assert_equals: expected "text/plain;charset=UTF-8" but got "text/plain;charset=\" UTF-8\"" FAIL charset in double quotes with backslashes that is UTF-8 does not change assert_equals: expected "text/plain;charset=\"u\\t\f-8\"" but got "text/plain;charset=\"UTF-8\"" FAIL unknown parameters need to be preserved assert_equals: expected "yo/yo;charset=UTF-8;yo=YO;x=y" but got "YO/yo;charset=UTF-8;yo=YO; X=y"
I think that the challenge here is to separate cases that have the potential of fixing or breaking websites from irrelevant subtests.
Created attachment 363117 [details] Patch
Comment on attachment 363117 [details] Patch Attachment 363117 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11306680 New failing tests: http/tests/xmlhttprequest/request-encoding2.html
Created attachment 363123 [details] Archive of layout-test-results from ews102 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 363126 [details] Patch
Created attachment 363208 [details] Patch
Comment on attachment 363208 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363208&action=review > Source/WebCore/platform/network/ParsedContentType.cpp:351 > + m_parameterValues.set("charset", charset); s/"charset"/"charset"_s/ Could also be String&&/WTFMove > Source/WebCore/xml/XMLHttpRequest.cpp:77 > static void replaceCharsetInMediaType(String& mediaType, const String& charsetValue) Can we make replaceCharsetInMediaType closer to spec by no longer passing charsetValue, since it is always UTF-8? We could then change replaceCharsetInMediaType prototype so that it takes a const String& and return a String. > Source/WebCore/xml/XMLHttpRequest.cpp:79 > + Optional<ParsedContentType> parsedContentType = ParsedContentType::create(mediaType); s/Optional<ParsedContentType>/auto/
Created attachment 363318 [details] Patch
Comment on attachment 363208 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363208&action=review >> Source/WebCore/platform/network/ParsedContentType.cpp:351 >> + m_parameterValues.set("charset", charset); > > s/"charset"/"charset"_s/ > > Could also be String&&/WTFMove Done. >> Source/WebCore/xml/XMLHttpRequest.cpp:77 >> static void replaceCharsetInMediaType(String& mediaType, const String& charsetValue) > > Can we make replaceCharsetInMediaType closer to spec by no longer passing charsetValue, since it is always UTF-8? > We could then change replaceCharsetInMediaType prototype so that it takes a const String& and return a String. Good idea, done. >> Source/WebCore/xml/XMLHttpRequest.cpp:79 >> + Optional<ParsedContentType> parsedContentType = ParsedContentType::create(mediaType); > > s/Optional<ParsedContentType>/auto/ Done.
Comment on attachment 363318 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363318&action=review > Source/WebCore/xml/XMLHttpRequest.cpp:77 > +static String replaceCharsetInMediaType(const String& mediaType) I would rename it to replaceCharsetInMediaTypeIfNeeded. Thinking about it, chances are that mediaType will be returned without any change. We could take a String&& and return a String. Or go back to void/String&. > Source/WebCore/xml/XMLHttpRequest.cpp:84 > + return parsedContentType->serialize(); "text/plain; charset=UTF-8" should remain as is and not become "text/plain;charset=UTF-8", right? Can we add a test if we do not cover this case? > LayoutTests/imported/w3c/ChangeLog:8 > + Sync test and uodate test expectation (all PASSes). s/uodate/update/
Created attachment 363337 [details] Patch
Comment on attachment 363318 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363318&action=review >> Source/WebCore/xml/XMLHttpRequest.cpp:77 >> +static String replaceCharsetInMediaType(const String& mediaType) > > I would rename it to replaceCharsetInMediaTypeIfNeeded. > Thinking about it, chances are that mediaType will be returned without any change. > We could take a String&& and return a String. Or go back to void/String&. I thought about it but thought the name would get kind of long. Anyway fine with it :) >> Source/WebCore/xml/XMLHttpRequest.cpp:84 >> + return parsedContentType->serialize(); > > "text/plain; charset=UTF-8" should remain as is and not become "text/plain;charset=UTF-8", right? > Can we add a test if we do not cover this case? We pass that, I tried it locally. I would like to cover this case in send-content-type-charset.htm, but in a follow-up commit to WPT. >> LayoutTests/imported/w3c/ChangeLog:8 >> + Sync test and uodate test expectation (all PASSes). > > s/uodate/update/ Fixed.
Comment on attachment 363337 [details] Patch Clearing flags on attachment: 363337 Committed r242284: <https://trac.webkit.org/changeset/242284>
All reviewed patches have been landed. Closing bug.
<rdar://problem/48521015>