-
Notifications
You must be signed in to change notification settings - Fork 155
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
Header-case and duplicate headers #21
Comments
Thanks for the write-up, @jpillora. Sorry, @peterjosling that you ran into an issue due to header casing. Since HTTP headers should be treated case insensitive per spec, I believe we’ll have to satisfy two constraints:
One idea is to save a map of the original headers for sending it to the server and one for keeping a normalized map (e.g. lowercase) for case insensitive retrieval. One edge case is allowing multi-value headers where the header name is set using different cases. Maybe the last one wins then, e.g. |
Yep, looks good. The hard part is making this work with the XHook
And also if we go with the last key, the prior key would need to be deleted. I think I'll go with the latter, since I'm hesitant about changing the existing An alternative to a second map, would to simply scan the existing map on all |
Unless there any objections, will try to get this in tonight |
@jpillora Just to clarify, are you talking about how to support multi-value headers with Documentation: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest#setRequestHeader() Re: preserving case for getResponseHeader = (name) ->
values = (value for header, value of headers when name.toLowerCase() is header.toLowerCase())
values.join ',' |
@gasi Yep, that will be the new
The |
Thanks for the outline—LGTM 👍 Not 100% sure about string vs array though. I can see that I know this isn’t Java, but I found Netty’s HTTP message implementation quite elegant:
Just 🍔 for 💭 😄 |
jpillora/xdomain#95 is actually an XHook issue, so we're continuing that thread here.
Currently XHook only allows unique headers and forces lowercase. According to the HTTP spec (HTTP 1.1 Section 4.2):
So XHook needs to be updated to reflect this
The text was updated successfully, but these errors were encountered: