[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-22 Thread James Rutherford
James Rutherford added the comment: Updated docs look good to me, thanks! -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-22 Thread R. David Murray
R. David Murray added the comment: Thanks James and Demien. I decided to rewrite the 'request' docs for Python3 to make them easier to follow (and more accurate). Hopefully I didn't make any mistakes, but you might want to review it just in case. -- resolution: -> fixed stage: commi

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-22 Thread Roundup Robot
Roundup Robot added the comment: New changeset 5c3dc817ffd7 by R David Murray in branch '2.7': #23539: Set Content-Length to 0 for PUT, POST, and PATCH if body is None. https://hg.python.org/cpython/rev/5c3dc817ffd7 New changeset f6f72422df96 by R David Murray in branch '3.4': #23539: Set Conten

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-19 Thread Demian Brecht
Demian Brecht added the comment: One super minor comment left in Rietveld (py3 patch). Otherwise LGTM. -- ___ Python tracker ___ ___ P

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-19 Thread Berker Peksag
Changes by Berker Peksag : -- nosy: +berker.peksag ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-19 Thread James Rutherford
James Rutherford added the comment: Updated 2.7 patch attached. -- Added file: http://bugs.python.org/file38570/issue23539-py27.patch ___ Python tracker ___ _

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-19 Thread James Rutherford
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 wel

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-13 Thread James Rutherford
James Rutherford added the comment: Ok I'll have a go at a consolidated python3 patch tomorrow. -- ___ Python tracker ___ ___ Python-b

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-13 Thread R. David Murray
R. David Murray added the comment: It would...I've started twice to do the commit and gotten interrupted both times. Having a patch I can just apply would help. If you would be willing to also generate a patch for python3, that would be a help as well. -- ___

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-13 Thread James Rutherford
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. -- ___

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-04 Thread James Rutherford
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 :) --

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-03 Thread R. David Murray
R. David Murray added the comment: Single patch, yes, but FYI we actually prefer a python3 patch against default as the single patch. (Unless there are major differences, which there aren't in this case.) I've made some minor review comments. You can either ack my changes (or disagree :) an

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-02 Thread Demian Brecht
Changes by Demian Brecht : -- stage: patch review -> commit review ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscrib

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-02 Thread Demian Brecht
Demian Brecht added the comment: > OK, sounds like we're approaching consensus? And I believe that the patch > as-is captures that consensus Yes, I believe that we’ve reached a consensus and that your patch captures it. Thanks for the work! -- ___

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-02 Thread Senthil Kumaran
Senthil Kumaran added the comment: One patch should be enough, @James. The committer will take care of porting. Thanks! ( I am yet to completely read the discussion/ review the patch and I will do that shortly.) -- ___ Python tracker

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-02 Thread James Rutherford
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

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-02 Thread R. David Murray
R. David Murray added the comment: Not to get into an API argument, but rather to convey some Python philosophy as I understand it: there *is* often a distinction in Python between the value 'None' and a boolean-False-value like ''. It is often the case in Python APIs that None means somethin

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-02 Thread Demian Brecht
Demian Brecht added the comment: > Aside from that, however, I see request.('GET', '/') and request.('GET', '/', > '') as clearly *different* from an API call standpoint, so I would in any > case preserve the existing behavior. I do understand the case that the the two examples look different

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-02 Thread R. David Murray
R. David Murray added the comment: Don't break backward compatibility. It's not like this was reported as a bug that caused a problem for real world code, it is about theoretical consistency. The risk of breaking someone code is much higher than the benefit of any such consistency, when talk

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-01 Thread Demian Brecht
Demian Brecht added the comment: +David, Senthil: Thoughts on this? -- nosy: +orsenthil, r.david.murray ___ Python tracker ___ ___ Pyt

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-01 Thread Demian Brecht
Demian Brecht added the comment: > but I wonder if that goes beyond the scope of this issue? I think it’s worthwhile to fix it while you’re already working on the logic there. I’d consider Content-Type being set for methods not expecting it as a (very) minor bug and it’s better to clear up the

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-03-01 Thread James Rutherford
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 th

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-28 Thread Demian Brecht
Demian Brecht added the comment: > I actually consider this a fix for the fix in 14721, rather than a new > feature. +1 -- ___ Python tracker ___ __

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-28 Thread Demian Brecht
Demian Brecht 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. Although I understand your thinking

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-28 Thread James Rutherford
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 bugfi

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-28 Thread Martin Panter
Martin Panter added the comment: Yes I agree with the behaviour that None means no body (for requests such as GET), and an empty string means an empty but present body. Correct me if I’m wrong, but I think the fix for Issue 14721 already does that. Perhaps the new behaviour needs a “Changed in

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-28 Thread James Rutherford
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

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-27 Thread Demian Brecht
Demian Brecht added the comment: > Also, I think OPTIONS should be removed from the list of methods that enforce > a Content-Length. I wouldn’t normally expect any payload for OPTIONS, since > RFC 7231 explicitly says it does not define a use for a payload, but requires > a Content-Type if a p

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-27 Thread James Rutherford
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

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-27 Thread Martin Panter
Martin Panter added the comment: I like Demian’s “expecting_len” technique slightly better because it would avoid calling None.fileno() and len(None), as Demian just said. Also, I think OPTIONS should be removed from the list of methods that enforce a Content-Length. I wouldn’t normally expect

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-27 Thread Demian Brecht
Demian Brecht added the comment: > 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. I see we definitely have similar thinking on the modifications required for this, but I don’t think

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-27 Thread Demian Brecht
Demian Brecht added the comment: My concern about this is around the combination of the following two passages: draft-ietf-httpbis-p2-semantics-14, Section 7.3: > Bodies on GET requests have no defined semantics. Note that sending > a body on a GET request might cause some existing impleme

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-27 Thread James Rutherford
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

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-27 Thread James Rutherford
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

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-27 Thread James Rutherford
James Rutherford added the comment: OK, thanks. -- versions: -Python 3.2, Python 3.3 ___ Python tracker ___ ___ Python-bugs-list mail

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-27 Thread Demian Brecht
Demian Brecht added the comment: > I'm assuming this affects all Python 3.X versions but I've specifically > encountered it on Python 2.7. Unless there are any core dev objections, I think it's applicable to 2.7, 3.4 and 3.5 as other minor 3.x versions are in security mode (https://docs.pytho

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-27 Thread James Rutherford
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 ___

[issue23539] Content-length not set for HTTP methods expecting body when body is None

2015-02-27 Thread Demian Brecht
New submission from Demian Brecht: #14721 solved setting the Content-Length header for 0-length bodies. However, it doesn't account for cases where body is None (reported by James Rutherford here: http://bugs.python.org/issue14721#msg236600). One method of solving this might be something like