-
Notifications
You must be signed in to change notification settings - Fork 43
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
Client.request bypassing urllib2.Request header normalization #10
Comments
A clarification. The resource server does handle content-type in a case insensitive way. The problem is that urllib2 adds the header Content-type with a value of application/x-www-form-urlencoded if not present in the set of headers. In other words, there is code in urllib2 which expects headers to already be normalized a does lookup using those normalized header names. See AbstractHTTPHandler. do_request_ (do_open in older versions) in http://svn.python.org/projects/python/branches/release27-maint/Lib/urllib2.py In other words, I was wrong about how my resource server was behaving. There are still unexpected effects from not using normalized request headers. |
Yeah, unfortunately there's definitely some magic that happens in various places in urllib. This behaviour is also consistent in 3.4. I guess I can kind of see the reasoning behind providing a sane default, but I don't like the assumption that if Unfortunately, I don't think there's anything on this end that I'm willing to do in order to prevent this from happening. The only thing that I can think of changing is to provide alternative defaults before calling into I'm going to close this as I don't think there's anything that can be done from this end. However, it's definitely good to have logged should anyone else encounter this problem, so thanks for reporting it! |
Since the request object is manipulating the user-supplied headers in its constructor, maybe we should consider Request.headers more like private data of the Request object. What about calling Request.add_header for each key/value in the user-supplied headers? transport_headers would become something like:
Alternatively, you could pass the set of headers into the request:
The final option is to not accept headers at all and ask that callers manipulate the Request object themselves (by wrapping transport_* or something). I wonder if one of these options is the "least surprising" to users of sanction. |
Here are my thoughts on the two issues raised here: Using add_header or Request(headers={}) IMHO, the behaviour in Default Content-Type header ( This is the behaviour of the underlying |
I agree that headers should be insensitive. I agree that what is put into the header dict should be what is sent over the wire. I agree that I am using the implementation of |
I found this because we happen to have a server which barfs on Content-Type but accepts Content-type. Go figure.
When a header dictionary is passed to urllib2.Request, each key/value of that dictionary is not blindly added to the request headers dictionary. Instead, request.add_header is called on each pair and that performs key.capitalize().
By simply calling req.headers.update after creating the request, sanction skips this header normalization.
Both transport_headers and transport_query suffer from this.
It probably makes sense for Client.request to mirror urllib2.Request as much as possible since it's trying to just be a thin wrapper. This is one inconsistency.
I am working around this issue by passing Content-type instead of Content-Type to Client.request.
I imagine that the default header normalization might cause problems as well (but, honestly, resource servers should be processing headers in a case insensitive way and it sucks that the one we have isn't). I can understand if you deem this not worth fixing. At the very least, I wanted to document the problem in case someone else runs into it.
The text was updated successfully, but these errors were encountered: