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
httplib doesn't specify content-length header for POST requests without data #58926
Comments
httplib doesn't specify the HTTP header 'content-length' for POST requests without data. Conceptually this makes sense, considering the empty content. However, IIS (7.5) servers don't accept such requests and respond with a 411 status code. See this question on StackOverflow for reference: http://stackoverflow.com/questions/5915131/can-i-send-an-empty-http-post-webrequest-object-from-c-sharp-to-iis. See also issue #223 of the Requests project, https://github.com/kennethreitz/requests/issues/223, which regards this problem in the context of the requests Python library. The following code makes a data-less POST request to the HTTP sniffer Fiddler running on localhost: Fiddler reports that it receives the following headers for the POST request, as you can see 'content-length' is not included: |
Could you provide a patch? Does this affect 3.x too? |
I can look into patch and 3.x tonight I think. Should I provide a test with an eventual patch? |
Patch with test, please :-). I know it is a pain in the ass, but the result is having a higher quality python. |
Python 3.2.3 on Windows Vista 64bit gives the same output for import http.client
conn = http.client.HTTPConnection('localhost',8888)
conn.request("POST", "/post", "", {})
conn.close() |
Which HTTP methods should we auto-define content-length for? POST and PUT? I noticed that my first attempt at a patch would define content-length also for GET requests, which broke a unit test (test_ipv6host_header). |
Actually, when inspecting the HTTP requests sent by Chrome for the different methods (a great little Chrome app called Postman let me fire requests manually), I found that content-length would be set for most methods. I could confirm that 'content-length: 0' would be set for the following methods: POST, PUT, PATCH, DELETE and HEAD. I guess it should be good enough to model that behaviour in httplib? |
HEAD?. It doesn't make sense in HEAD if it doesn't make sense in GET. Looking around, I found this, to mud the water a little bit more: <http://fixunix.com/tcp-ip/66198-http-rfc-related-question-content-length-0-get-request.html\>. Not being explicit about this is a "bug" in the specification, I think. |
Here's my initial proposal for a patch against httplib, based on the 2.7 branch of cpython repository. I've included a couple of tests, which check that when content is empty, content-length is set to 0 for certain methods (POST/PUT etc) and not specified for others. |
Yes, I agree it doesn't make much sense for HEAD AFAICT, but Chrome does it. Maybe there's a reason? |
The rule for content-length seems, if there is a body for a request, even if the body is "" ( empty body), then you should send the Content-Length. The mistake in the Python httplib was, the set_content_length was called with this condition. if body and ('content-length' not in header_names): If the body was '', this was skipped. The default for GET and methods which do not use body was body=None and that was statement for correct in those cases. A simple fix which covers the applicable methods and follows the definition of content-length seems to me like this:
+ if body is not None and 'content-length' not in header_names: I prefer this rather than checking for methods explicitly as it could go into unnecessary details. (Things like if you are not sending a body why are you sending a Content-Length?. This fails the definition of Content-Length itself). The Patch is fine, I would adopt that for the above check and commit it all the active versions. Thanks Arve Knudsen, for the bug report and the patch. |
New changeset 57f1d13c2cd4 by Senthil Kumaran in branch '2.7': New changeset 6da1ab5f777d by Senthil Kumaran in branch '3.2': New changeset 732d70746fc0 by Senthil Kumaran in branch 'default': |
This is fixed in all the branches. Thanks! |
Too late for asking to keep the parenthesis :-). I hate to have to remember non-obvious precedence rules :-). Cognitive overhead. |
The fix for this still doesn't set Content-Length to zero when body is None, but I don't see any reason why this should be the case. For example, the following snippet would work for any 'empty' body: if 'content-length' not in header_names:
self._set_content_length(body if body is not None else '') I'm happy to produce a patch if there's any chance it would be merged. |
If the patch adheres to the RFC, then I see no reason why it shouldn't be merged. What makes this a little more tricky than the snippet that you included in your post though (which would include the Content-Length header for all HTTP methods) is the following from RFC 7230: A user agent SHOULD send a Content-Length in a request message when Currently, there is nothing in the http package that defines whether or not a given HTTP method expects a body (as far as I'm aware at any rate), although this would be a simple addition. I'd imagine that the result might look like this: _METHODS_EXPECTING_BODIES = {'OPTIONS', 'POST', 'PUT', 'PATCH'} if method.upper() in _METHODS_EXPECTING_BODIES and \ I'd prefer to have the conversion from None to empty string done in the body of _set_content_length in order to ensure consistency should the call be made from elsewhere. |
James, Demian: this issue has been closed for almost three years and the changes released long ago: comments made here will likely be ignored. Please open a new issue if you want them to be acted on. |
Thanks for the heads up Ned. James: I've created bpo-23539 in the event that you'd like to contribute a patch. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: