Header Response not case insensitive

[dallasrpi]

Its pretty common to have headers use "Content-Length" or "Content-length" or "Etag" vs "ETag" . This presents a problem when you don't know which version is coming down the pike. An obvious work around is to iterate the headers object and lowercase the keys but just noting it makes this feature a little less usable.

[hanshasselberg]

Hi @dallasrpi,
thanks for reporting. I'll think about that one. Do you know what other http client libraries are doing?

[hanshasselberg]

Ok, net/http downcases it: https://github.com/ruby/ruby/blob/trunk/lib/net/http/header.rb#L17.

[ethicalhack3r]

+1 for downcasing the response headers.

On Wed, Oct 31, 2012 at 3:34 PM, Hans Hasselberg
notifications@github.comwrote:

Ok, net/http downcases it:
https://github.com/ruby/ruby/blob/trunk/lib/net/http/header.rb#L17.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/227#issuecomment-9946839.

[hanshasselberg]

Convinced.

[hanshasselberg]

Fixed in 256c954.

[dallasrpi]

Awesome thanks for your contributions!

[myronmarston]

This seems like a pretty big backwards-incompatible change. Code that worked fine with 0.5.0 (using something like response.headers['Set-Cookie']) will now fail because it will get a nil value since the header key is now set-cookie. This isn't just a theoretic failure; this broke VCR's specs.

According to the principles of Semantic Versioning, you should only make backwards-incompatible changes in major releases, so if you care to follow those principles, you should release another 0.5.x release with a fix back to the old behavior and then you can release this in 1.0.

If you want to provide case-insensitive access to get headers (which is a useful feature), there are ways to get it without breaking backwards-incompatibility; for example, you could change response.headers so that it returns an object that presents the hash interface but internally normalizes keys to a common case when accessing an individual key. That way, old code that used Set-Cookie could continue to work, and set-cookie could work fine as well.

[myronmarston]

I came up with a fix to get VCR green again, but it's a hack more than anything else. There are a couple of issues at play here:

• Request headers are not normalized in this fashion but response headers are. That seems inconsistent. They should be treated the same, I think.
• How does this work with a mock response? It looks like headers are only downcased when parsing a real raw HTTP response, so if a mock response is created the headers will not be normalized in this fashion. Is the burden on VCR/WebMock and users of the mocking API to downcase headers to make things consistent? If you're committed to this change, Typhoeus needs to ensure that the headers API is consistently lower case headers, regardless of how the headers enter typhoeus.

In the future, please consider running the VCR and WebMock test suites before cutting typhoeus releases; they serve as nice regression suites to make sure you haven't unintentionally broken backwards compatibility and would have surfaced these issues so we could work out the details before this API became public.

[myronmarston]

BTW, here's one way to make a hash that keeps the original casing (and thus, remains backwards-compatible) while allowing case-insensitive access:

def case_insensitive_hash(hash)
  normalized_hash = hash.each_with_object({}) do |(k, v), h|
    h[k.downcase] = v
  end

  case_insensitive = hash.dup
  case_insensitive.default_proc = Proc.new do |h, k|
    normalized_hash[k.downcase]
  end

  case_insensitive
end

Essentially, it sets the hash's default_proc so that it looks up the downcased key in a normalized hash if not entry is found for a given key....so it falls back to the lowercase when needed for case-insensitivity. When you inspect the hash, the original casing is retained.

[hanshasselberg]

@myronmarston thanks for looking into it. I fixed this bug in ced7fc7.

I try to stick to semantic versioning and didn't intend to introduce a backwards incompatibility. But I did follow it:

1. Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.

I totally agree with your first comment nevertheless!
Regarding your second comment:

1. I normalize the response headers because the spelling is not under your control. It is for request headers. RFC.
2. I think Typhoeus should handle the mock response case.

Regarding your third comment:

Thanks, I'll check it out!

[hanshasselberg]

In the future, please consider running the VCR and WebMock test suites before cutting typhoeus releases; they serve as nice regression suites to make sure you haven't unintentionally broken backwards compatibility and would have surfaced these issues so we could work out the details before this API became public.

Will do that from now on!

[findchris]

So, not sure if this is related, but upgrading from typhoeus 0.4.2 to 0.5.3 has broken the following code in my app:

response = Typhoeus::Request.new("www.example.com").run
content_type = response.headers_hash[:content_type]
location = response.headers_hash[:location]

Previously, these would return individual header values as expected. Now, both statements above return the same thing (a hash):

{"Location"=>"http://www.iana.org/domains/example/", "Server"=>"BigIP", "Connection"=>"Keep-Alive", "Content-Length"=>"0"}

Any help?

[hanshasselberg]

@findchris headers_hash should return an hash: https://github.com/typhoeus/typhoeus/blob/master/lib/typhoeus/response/informations.rb#L187.

[findchris]

Right, but previously that code worked correctly, and I was able to retrieve the desired values from the hash using those symbolized, lowercase keys.

-Chris

On Jan 7, 2013, at 12:44 AM, Hans Hasselberg notifications@github.com wrote:

@findchris headers_hash should return an hash: https://github.com/typhoeus/typhoeus/blob/master/lib/typhoeus/response/informations.rb.

—
Reply to this email directly or view it on GitHub.

[findchris]

Further illustration:

1.9.2p320 :013 > response.headers_hash
 => {"Location"=>"http://www.iana.org/domains/example/", "Server"=>"BigIP", "Connection"=>"Keep-Alive", "Content-Length"=>"0"} 
1.9.2p320 :014 > response.headers_hash[:location]
 => {"Location"=>"http://www.iana.org/domains/example/", "Server"=>"BigIP", "Connection"=>"Keep-Alive", "Content-Length"=>"0"} 
1.9.2p320 :015 > response.headers_hash["location"]
 => "http://www.iana.org/domains/example/" 

So: Getting a value by symbol returns the full headers_hash (quite unexpected!) but getting a value by string (case-insensitive) works.

[findchris]

@i0rek I can definitely switch from symbols to string keys, but it is just surprising that symbols once worked and now they don't.

[findchris]

Actually, simply using string keys doesn't behave as expected either:

1.9.2p320 :020 > response.headers['content-type']
 => {"Location"=>"http://www.iana.org/domains/example/", "Server"=>"BigIP", "Connection"=>"Keep-Alive", "Content-Length"=>"0"} 

It seems that when a header key is not recognized, it simply returns the entire headers hash, which is definitely unexpected as one would instead expect nil. Do you agree?

[hanshasselberg]

@findchris this is not far away from expected behavior!

[findchris]

@i0rek What am I missing here? If the hash does not contain the specified key, it returns itself (a hash). That is not surprising? Following Ruby hash semantics, a nil is expected.

My point is that this behavior has changed drastically between 0.4.2 and 0.5.3. Symbols as keys used to work, and if the key was not present, nil was returned. Is nil ever returned from the headers hash?

[hanshasselberg]

@findchris so sorry - this was a typo! you are right!! it is very surprising!

[findchris]

@i0rek Anything I can do to help? Are symbolized keys not acceptable any more?

[findchris]

@i0rek Good news: progress!

I was viewing the source for header.rb in master (https://github.com/typhoeus/typhoeus/blob/master/lib/typhoeus/response/header.rb). I noticed the @sanitized instance variable, which is absent in 0.5.3. Using the header.rb from master at least makes this snippet behave as expected:

1.9.2p320 :008 > response.headers['does_not_exist']
 => nil 

So, this issue is fixed in master. Can you release 0.5.4?

The only remaining issue is that symbols no longer work as headers hash keys. If this is desired, I'll update my code to use string keys.

Cheers.

[hanshasselberg]

@findchris oh this is awesome! I would love to release 0.5.4 but it fails on ruby 1.8.7. The problem is that there is no Hash#default_proc=(https://github.com/typhoeus/typhoeus/blob/master/lib/typhoeus/response/header.rb#L20) for 1.8.7 and I need to come up with a workaround. Do you want to take a stab at it?

[myronmarston]

@i0rek -- here's one way I've dealt with the lack of Hash#default_proc= on 1.8.7:

seomoz/interpol@be64cbf

You might be able to use the same technique.

[hanshasselberg]

Thanks - thats great!!

[findchris]

@i0rek Are you working on this or do you want me to?

[hanshasselberg]

@findchris Would be great if you could do that!

[findchris]

@i0rek Pull request is here: #259

Thanks

[findchris]

#259 has been merged. Thanks @i0rek.

Can you release 0.5.4 now?

[hanshasselberg]

@findchris here comes access via symbols: 0f9887d

[findchris]

@i0rek Excellent. Now the headers are totally compatible with 0.4.2. Cheers.

[hanshasselberg]

@findchris there you go! https://github.com/typhoeus/typhoeus/blob/master/CHANGELOG.md#054

[findchris]

Thanks. I'll try it out.

[ric2b]

This still happens with fetch(), would it make sense to reopen this issue or should I create a new one?