Skip to content
This repository has been archived by the owner on Dec 7, 2018. It is now read-only.

case-insensitivity for header field names #152

Merged
merged 2 commits into from
Jun 17, 2014

Conversation

kenichi
Copy link
Member

@kenichi kenichi commented Jun 17, 2014

while testing angelo on
heroku, i discovered that heroku's router is downcasing request
header keys. this is valid according to:

http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2

this change should make all access to request header keys
insensitive, though probably not in a very performant way.

there's most likely a way better way of handling this... but wanted to get this on the radar. this app actually works on heroku with the beta websocket support enabled. note the Gemfile.

while testing [angelo](http://github.com/kenichi/angelo) on
heroku, i discovered that heroku's router is downcasing request
header keys. this is valid according to:

http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2

this change should make all access to request header keys
insensitive, though probably not in a very performant way.
@ungoldman
Copy link

👍

@tarcieri
Copy link
Member

Note that Reel did something like this originally (actually it put headers in the canonical form instead of downcasing) but we removed it for performance reasons. See e.g. 9e0aeb2

I know Mongrel did something like this, and perhaps that's where Heroku got the idea from, but doing this eagerly is somewhat expensive, especially for headers that are never accessed.

@kenichi
Copy link
Member Author

kenichi commented Jun 17, 2014

i definitely understand the performance reasons, and i don't like the way rack upcases and prepends 'HTTP_' to everything; but, for situations like this where header keys are not actually required to be in a certain case, it doesn't work unless the case-insensitivity is there. i also don't like that heroku is doing that to the requests, but alas.

note that this does not eagerly change header keys; it should only run the block when a key is not found. so i think it's basically the difference between one more Hash being allocated by the merge, if the key is never accessed. in siege tests, it seems to drop about 150 transactions / sec.

@tarcieri
Copy link
Member

Okay, my bad, I clearly didn't read the code. This is a neat trick, and seems ok to me, thanks!

tarcieri added a commit that referenced this pull request Jun 17, 2014
case-insensitivity for header field names
@tarcieri tarcieri merged commit 96c6d00 into celluloid:master Jun 17, 2014
@kenichi kenichi deleted the case_header_keys branch November 27, 2016 17:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants