[PATCH] Non-normalizing HeaderHash with case insensitive lookups
Reported by Ryan Tomayko | December 28th, 2008 @ 09:23 AM | in 0.9
Rack::Utils::HeaderHash currently normalizes header names to a naive camel-case (e.g., "content-length" -> "Content-Length", "ETag" -> "Etag", "Content-MD5" -> "Content-Md5"). There's been various discussion on why a non-normalizing but case-insensitive implementation would be preferable:
The following patch makes HeaderHash preserve the case of the header name when set but has case insensitive lookup, replace, delete, and include semantics:
http://github.com/rtomayko/rack/...
All specs pass with the exception of the ones that tested for case normalization specifically.
Comments and changes to this ticket
-
chneukirchen (at gmail) December 28th, 2008 @ 01:18 PM
This is an important patch, we should apply it and change all instances of the code that depend on explicit casing (unfortunately lots...).
-
Ryan Tomayko December 28th, 2008 @ 08:56 PM
Let me run through and try to take a quick inventory on what's impacted. It should be limited to stuff that reads/tests response headers without first wrapping them in Rack::Response or a HeaderHash. Code that only sets headers shouldn't be effected.
It might not be too bad. Maybe. Hmmm..
-
Ryan Tomayko December 28th, 2008 @ 09:45 PM
So, it's not too bad. Here's the classes that access headers by name directly via headers Hash: ShowStatus, Deflator, ContentLength, and ConditionalGet. Everything else either does no header checking or uses Rack::Response, which uses the HeaderHash internally.
I should have a patch together shortly.
-
Ryan Tomayko December 29th, 2008 @ 10:27 AM
Okay, I've updated my "headernorm" branch so that all code that manipulates response headers uses HeaderHash:
http://github.com/rtomayko/rack/...
Here's the diff of just those changes:
http://github.com/rtomayko/rack/...
It was definitely a lot less than I originally thought. There's surprisingly little code that modifies response headers and some of it was already using Rack::Response.
-
Ryan Tomayko December 29th, 2008 @ 10:53 AM
- Milestone set to 0.9
- Assigned user set to Ryan Tomayko
-
Ryan Tomayko December 30th, 2008 @ 02:16 AM
- State changed from new to resolved
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป