HTTPHeaderDict in requests

[shazow]

Do we want to accept HTTPHeaderDict objects in request header fields interchangeably with dicts/lists of tuples? Any reason why not?

Right now if we do that, foo=bar headers get encoded as foo: ('Foo': 'bar')

• Tests
• Fix

Fixes #632

[shazow]

Looks like the problem is that dict(foo='bar') != dict(HTTPHeaderDict(dict(foo='bar'))). I think it's because Py27+ uses dict.viewitems() to clone which we're returning the wrong thing for.

>>> h = HTTPHeaderDict({'foo': 'bar'})
>>> h.viewitems()
dict_items([('foo', ('foo', 'bar'))])

/cc people who worked on HTTPHeaerDict @jschneier @ml31415 @seocam

[sigmavirus24]

I had a fix for this in my fork but we closed that PR in favor of @ml31415's header fixes. I don't think I have it lying around anymore either

[shazow]

@sigmavirus24 Hm are you saying that this was a known bug? I had no idea. :/ Any guidance on how you fixed it?

[ml31415]

The HTTPHeaderDict is implemented to preserve the case. Don't know where this get's messed up. I don't think it's headerdicts fault.

[Lukasa]

I think I agree with @shazow: we need to override viewitems but we kinda can't, because it's a C-level function and you just can't really override those. This is why subclassing from dict is a bad idea: this object should really subclass collections.MutableMapping.

[shazow]

@ml31415 It is headerdict's fault because viewitems returns the wrong thing. In order to support both lowercase and originalcase, we're storing things as self[lowercase_key] = (originalcase_key, value) which is what viewitems returns but should be returning the equivalent of self[originalcase_key] = value.

So we either need to override viewitems somehow (can't figure out how) or stop inheriting from dict and inherit from MutableMapping as before.

... Erm, what @Lukasa just said while I was writing this.

[ml31415]

Not 100% sure, but I suppose .viewitem could get overriden as well, just as the other methods can. Viewitem was not considered in the current implementation yet, true, but to see it from another point of view: Is there any special reason against simply using .items instead of .viewitems there?

[shazow]

@ml31415 It's not us deciding whether to use items or viewitems. It's the Python implementation. :)

a = dict(foo='bar')
b = dict(HTTPHeaderDict(a))
a == b # False, because b is {"foo": ("foo", "bar")}

[shazow]

This is basically what we'd want: https://github.com/kennethreitz/requests/blob/master/requests/structures.py#L14

[ml31415]

Well, creating a dict that way is just not a lossless operation otherwise. Use .itermerged instead.

[shazow]

@ml31415 Again, this is not something I'm doing. It's what would be required to support passing an HTTPHeaderDict into an httplib request and having it Work(tm).

[ml31415]

Not sure if we're understanding each other here, or talk about different things ... :) Any plain dict does not support duplicate header lines. The requests CaseInsensitiveHeaderDict also doesn't do that. I wouldn't expect requests to run with another header implementation without changes. One of them being, to call the necessary conversion function, i.e. .itermerged, before throwing it into something multiheader-unaware like httplib. If there should be something else, that would be required from HTTPHeaderDict, I'm sure it could be provided easily.

The ugly thing about it, iterating the whole headerdict for every http request would be quite a time waster, given that it's a quite rare case to send duplicate headers. As it wasn't implemented so far, no one seems to have missed it too much.

[sigmavirus24]

Y'all are talking past each other. This was an issue I was at least aware of. If we want to fix it, I'll try to get some time during the night this week to fix it

[shazow]

@sigmavirus24 Still up for taking a crack at this? Would love to get this fixed before the next release. (I think #670 is suffering from this too)

[atodorov]

@shazow ,
confirmed. #670 is a duplicate of this one and the problem really breaks an important piece of software for me - the python-bugzilla tools and module in particular.

Any idea if this can be fixed quickly or I need to go with a workaround in python-requests, essentially converting back to dict and then to their CaseInsensitiveDict ?

[shazow]

@atodorov Unfortunately it can only be fixed as quickly as it takes for somebody to fix it. You're welcome to take a crack at it if you have the time to spare, but otherwise I'd put in a workaround. I suggest using .items().

[atodorov]

@shazow - I'm on Python 2.7 so .items is not an option according to your previous comments. I will take additional look at it though.

[shazow]

@atodorov What's wrong with using .items() on Py27? The comments above refer to .viewitems() which is different.

[shazow]

@sigmavirus24 Any chance you're still up for tackling this soon? I want to do a major version release early next week, would love to get this in.

[shazow]

Closing in favour of #679.