-
Notifications
You must be signed in to change notification settings - Fork 984
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
Suggestion: return response header values as arrays since they can have multiple values #44
Comments
I'd support a backwards-incompatible change that makes env[:response_headers] always return headers with array values. It's crazy how inconsistent (and wrong) the behavior is in some of the existing adapters. |
Inconsistent—yes. But headers as strings are definitely more convenient to use. Imagine having to type I'm tempted to implement something like Net::HTTP headers, where you have to use a special method to access multivalues as arrays. Also, from the spec:
|
@mislav: Interesting. Thanks for quoting the spec. I need to read the RFCs more. I guess the encoding issue I mentioned for using a comma-separated values is a non-issue.
That might be worthwhile. |
I definitely don't want every header value to be an array. I hated that shit from ASP 3.0 :)
|
Seems like we agreed to keep comma-separated values as String. Closing… |
For what it's worth, this is an issue when multiple Set-Cookie headers are sent, since Set-Cookie contains a comma in the
I thought I would solve this by subclassing the Faraday NetHttp adapter or inserting a middleware, but unfortunately the fact that joining by comma is baked into This appears to be a known issue with the RFC and many client implementations, eg. Google AppEngine: http://code.google.com/p/googleappengine/issues/detail?id=3379 At this point, the only possible solution seems to be to manually parse the concatenated header using a regex that avoids splitting in the middle of the
|
For background, see issue 43.
Currently header values are strings. When the response contains multiple values for the same header, it is a comma separated string (as of the fix for issue 43).
I think it'd be nice if the header values were arrays so that multiple values are explicit. Using a comma-separated string has potential encoding issues: how does a user distinguish between a single value of a string containing a comma, and a header of multiple values?
As @mislav pointed out, changing mutli-value headers to an array could create confusion for the user if the values are strings sometimes (i.e. for singel values) and arrays other times. I'm not sure of the best idea for that, but one idea is to keep the current behavior for env[:response_headers], with a deprecation warning, and add a new hash entry (maybe env[:response_header_hash] or something) that returns a hash with arrays for all values. That would make things consistent (use all arrays), while not breaking existing users who depend on header values being strings.
The text was updated successfully, but these errors were encountered: