[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: Updated docs look good to me, thanks! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: Python 3 patch attached. The documentation has changed structure a little so I've adapted (simplified) this from the original. Otherwise, it's pretty much the same, except with python3 fixes, and incorporated feedback. I'll upload an updated 2.7 patch as well in a few minutes. -- Added file: http://bugs.python.org/file38569/issue23539-py3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: Updated 2.7 patch attached. -- Added file: http://bugs.python.org/file38570/issue23539-py27.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: Hi all, apologies for the spam, but I just wanted to confirm that no-one is waiting on anything from me... I'm happy to consolidate the final minor points make the patch against python3 if that would simplify things. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: Ok I'll have a go at a consolidated python3 patch tomorrow. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: Single patch against default makes sense and I'll do that in future. As for the review comments, I'm happy to go with all of your suggestions but have offered a tweak to the docstring that you can take or leave at your discretion :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: OK, sounds like we're approaching consensus? And I believe that the patch as-is captures that consensus, so should I proceed and make another for 3.X for review? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: My feeling is that '' implies present but empty (so should have a content-length set to zero), whereas None implies missing (so should only have a content-length header set to zero if the method is expecting a body. ... In light of that, I think that HTTPConnection(‘example.com’).request(‘GET’, ‘/‘, ‘’) and HTTPConnection(‘example.com’).request(‘GET’, ‘/‘) should result in identical headers with no Content-Length set. I get your reasoning here, but I wonder if that goes beyond the scope of this issue? My initial thinking was the same, but then realised that was inconsistent with the current behaviour. For example, a GET with '' in the body already sets content-length: 0, but a GET with None for the body doesn't set a content length at all. I haven't changed this behaviour in my patch, except for PUT, POST, and PATCH where None and '' are now equivalent for those methods. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: I actually consider this a fix for the fix in 14721, rather than a new feature. The only new behaviour here is setting content length to be zero if body is None on PATCH, POST, or PUT. Happy to change the labeling if that's the consensus but IMO it's a bugfix. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: Happy to remove OPTIONS from the list of methods that gets a content-length where body is None, but do we also want to consider behaviour if it's the empty string? My feeling is that '' implies present but empty (so should have a content-length set to zero), whereas None implies missing (so should only have a content-length header set to zero if the method is expecting a body. I've updated the patch with this logic, including ensuring proper explicit test coverage for the cases described. Docs updated too. -- Added file: http://bugs.python.org/file38276/issue23539-py27.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: Thanks for setting up the new issue, I'll cook up a patch. I'm assuming this affects all Python 3.X versions but I've specifically encountered it on Python 2.7. -- nosy: +jimr versions: +Python 2.7, Python 3.2, Python 3.3, Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: OK, thanks. -- versions: -Python 3.2, Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: Patch attached for the 2.7 branch, including updated tests. All tests pass. Let me know if this looks like a sensible approach and I'll produce something comparable for 3.X. The logic now is as it was before, except that we set a content length of zero if the body is None and the method is one of OPTIONS, PATCH, PUT, or POST. -- keywords: +patch Added file: http://bugs.python.org/file38267/issue23539-py27.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: OK, I've got a patch but it's failing on 'test_send_file'[1], which is sending a body on a GET request. According to the IETF memo[2]: Bodies on GET requests have no defined semantics. Note that sending a body on a GET request might cause some existing implementations to reject the request. So I don't know whether to remove the method constraint so this test passes, or to fix this test to use a different method. My feeling is that stripping out Content-Length for GET requests could cause problems for some code that for whatever reason uses this, so we should remove that constraint (it was only SHOULD NOT after all) and go back to the original proposal of just setting the content length for bodies that are None as well as empty strings. [1] https://hg.python.org/cpython/file/325aec842e3e/Lib/test/test_httplib.py#l297 [2] http://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-14#section-7.3 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23539] Content-length not set for HTTP methods expecting body when body is None
James Rutherford added the comment: The first patch should actually be modified so the condition reads (update attached): if body is None and method_expects_body: thelen = 0 elif body is not None: ... Demian, I believe this is equivalent to your 'expecting_len' proposal, I've just put the logic inside _set_content_length. My approach was to leave as much existing logic as possible in place to avoid the introduction of new bugs (first patch wasn't entirely successful in this!). As for OPTION, If the OPTIONS request includes a message-body (as indicated by the presence of Content-Length or Transfer-Encoding), then the media type MUST be indicated by a Content-Type field. Although this specification does not define any use for such a body, future extensions to HTTP might use the OPTIONS body to make more detailed queries on the server. From http://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-14#section-7.2 -- Added file: http://bugs.python.org/file38270/issue23539-py27.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23539 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue14721] httplib doesn't specify content-length header for POST requests without data
James Rutherford added the comment: 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. -- nosy: +jimr ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue14721 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
