New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
XMLHttpRequest: make various header tests much more strict #5008
Conversation
Notifying @Manishearth, @Ms2ger, @caitp, @emilio, @hallvors, @ibelem, @jdm, @jungkees, @kangxu, @mathiasbynens, @ronkorving, and @wisniewskit. (Learn how reviewing works.) |
Firefox (nightly channel)Testing web-platform-tests at revision b1b5fa4ceefbbf87a8eb27b335cb265218ff50f2 All results12 tests ran/XMLHttpRequest/anonymous-mode-unsupported.htm
/XMLHttpRequest/open-after-setrequestheader.htm
/XMLHttpRequest/open-referer.htm
/XMLHttpRequest/preserve-ua-header-on-redirect.htm
/XMLHttpRequest/send-accept-language.htm
/XMLHttpRequest/setrequestheader-allow-empty-value.htm
/XMLHttpRequest/setrequestheader-allow-whitespace-in-value.htm
/XMLHttpRequest/setrequestheader-case-insensitive.htm
/XMLHttpRequest/setrequestheader-content-type.htm
/XMLHttpRequest/setrequestheader-header-allowed.htm
/XMLHttpRequest/setrequestheader-header-forbidden.htm
/XMLHttpRequest/setrequestheader-open-setrequestheader.htm
|
Chrome (unstable channel)Testing web-platform-tests at revision b1b5fa4ceefbbf87a8eb27b335cb265218ff50f2 All results12 tests ran/XMLHttpRequest/anonymous-mode-unsupported.htm
/XMLHttpRequest/open-after-setrequestheader.htm
/XMLHttpRequest/open-referer.htm
/XMLHttpRequest/preserve-ua-header-on-redirect.htm
/XMLHttpRequest/send-accept-language.htm
/XMLHttpRequest/setrequestheader-allow-empty-value.htm
/XMLHttpRequest/setrequestheader-allow-whitespace-in-value.htm
/XMLHttpRequest/setrequestheader-case-insensitive.htm
/XMLHttpRequest/setrequestheader-content-type.htm
/XMLHttpRequest/setrequestheader-header-allowed.htm
/XMLHttpRequest/setrequestheader-header-forbidden.htm
/XMLHttpRequest/setrequestheader-open-setrequestheader.htm
|
@@ -15,7 +15,7 @@ | |||
client.open("POST", "resources/inspect-headers.py?filter_name=X-Empty", false) | |||
client.setRequestHeader('X-Empty', value) | |||
client.send(null) | |||
assert_equals(client.responseText, 'x-empty: '+ String(value.trim()).toLowerCase()+'\n' ) | |||
assert_equals(client.responseText, 'X-Empty: ' + String(value.trim()) + '\n' ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: don't need the String() here anymore
Unfortunately as established in whatwg/xhr#108 setRequestHeader() uses `, ` whereas fetch() uses `,` as value separator. This introduces a legacySpaceFlag for combine that XMLHttpRequest and WebSocket can use. New code and CORS (in Access-Control-Request-Headers) can continue not passing this flag. Tests were fixed in web-platform-tests/wpt#5008.
In particular, setRequestHeader() should use 0x2C 0x20 as separator (not just 0x2C) and get(All)ResponseHeader(s)() should do so too. The latter also always needs to end in 0x0D 0x0A rather than omitting it at the end. This depends on whatwg/fetch#504 landing first. Tests: web-platform-tests/wpt#4641 and web-platform-tests/wpt#5008. Fixes #108 and fixes #109.
In particular, setRequestHeader() should use 0x2C 0x20 as separator (not just 0x2C) and get(All)ResponseHeader(s)() should do so too. The latter also always needs to end in 0x0D 0x0A rather than omitting it at the end. This depends on whatwg/fetch#504 landing first. Tests: web-platform-tests/wpt#4641, web-platform-tests/wpt#5008, and web-platform-tests/wpt#5115. Fixes #108 and fixes #109.
Fixes #2612 and fixes #4377.
Also fixes part of #4641.