Revise SPEC to not require #each on header values (Ruby 1.9)
Reported by Ryan Tomayko | February 1st, 2009 @ 07:43 AM | in 1.0
The SPEC currently requires response header values to respond to
#each
:
The values of the header must respond to each. The values passed on
each must be Strings and not contain characters below 037.
This needs to be revised so that Ruby 1.9 Strings are legal header values.
One solution is to simply change the SPEC to use "each_line" instead of "each":
The values of the header must respond to each_line. The values passed on
each_line must be Strings and must not contain characters below 037.
This makes Strings valid header values.
This breaks cases where an Array was used for header values.
Code that used an Array for a header value should be modified to
join the Array with 012 ("\n"
) instead.
Comments and changes to this ticket
-
Ryan Tomayko February 1st, 2009 @ 09:00 AM
One issue with this approach is that
String#each_line
returns the trailing line-end on each string it yields, which breaks the "must not contain characters below 037" constraint. Handlers and other multi-value consuming code will need to rstrip each value produced byeach_line
. -
Ryan Tomayko February 1st, 2009 @ 09:14 AM
Here's a proposed implementation (top three commits):
http://github.com/rtomayko/rack/...
The full suite of specs pass (i.e.,
rake fulltest
).Here's a summary of changes:
Rack::Lint
has been updated to adjust the SPEC.Rack::Utils::HeaderHash
(and henceRack::Response
) allowsArray
values for headers. Those values are converted to Strings withjoin("\n")
when converted to a true hash with#to_hash
.- All handlers have been modified to call
#each_line
instead of#each
on header values and torstrip
each String value.
There's room for improvement here but I wanted to get a concrete proposal in place before starting a thread on the ML.
-
Ryan Tomayko February 2nd, 2009 @ 10:20 AM
I put together another proposal based on the following SPEC text suggested by chris2:
The values of the header must be Strings, consisting of lines (for multiple header values) seperated by "\n". The lines must not contain characters below 037.
Branch is "headerspec2" in my rack repo:
http://github.com/rtomayko/rack/...
Commits 96f8b7d..be8f4ff:
commit be8f4ffdba31037082af28e3b984d6db72bdf927 Date: Sun Feb 1 00:57:22 2009 -0800 Handlers split header values on "\n" instead of using #each commit 826d080bc831222137cc1effef0eb0a69d9e367e Date: Sun Feb 1 00:48:58 2009 -0800 Normalize Array header values to Strings in HeaderHash#to_hash commit 96f8b7de1e3f5a714f1f2bf0694dd423108bcee6 Date: Sun Feb 1 00:17:36 2009 -0800 SPEC: header values must be Strings instead of responding to #each [#27]
Please review.
-
Scytrin dai Kinthra February 5th, 2009 @ 06:02 PM
On commit 96f8b7de1, I'd consider rewording: "header values must be Strings, but " to "a header value must be a String, but "
After that, LGTM
-
Ryan Tomayko February 6th, 2009 @ 08:40 PM
- State changed from new to resolved
Merged in 732a4fa with Scytrin's suggested changes. Thanks for the review everyone.
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 ยป