[issue25738] http.server doesn't handle RESET CONTENT status correctly

Changes by Martin Panter : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker ___ ___

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Roundup Robot added the comment: New changeset 1dc495007b8f by Martin Panter in branch '3.5': Issue #25738: Don’t send message body for 205 Reset Content https://hg.python.org/cpython/rev/1dc495007b8f New changeset b1041ddb1391 by Martin Panter in branch '2.7': Issue #25738: Don’t send message b

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Susumu Koshiba added the comment: A patch for 3.5 attached. -- Added file: http://bugs.python.org/file43292/issue25738_http_reset_content_3.5_02.patch ___ Python tracker ___ ___

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Susumu Koshiba added the comment: Great, thanks for checking. Attaching patch for 2.7. 3.5 will follow. -- Added file: http://bugs.python.org/file43291/issue25738_http_reset_content_2.7_02.patch ___ Python tracker

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Martin Panter added the comment: Patch 07 looks fine. I presume you still want to do the porting to 3.5 and 2.7. -- ___ Python tracker ___ ___

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Susumu Koshiba added the comment: Thanks for the suggestions, I've updated the test cases and created a patch for 3.6. -- Added file: http://bugs.python.org/file43277/issue25738_http_reset_content_07.patch ___ Python tracker

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Martin Panter added the comment: Thanks Susumu, I think the logic is right this time. I left some new suggestions for the tests, if you want to take them on or not, that’s up to you. -- ___ Python tracker

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Changes by Mathieu Dupuy : -- nosy: -deronnax ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.pyt

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Susumu Koshiba added the comment: Ah yes, you are right. I didn't think about the cases where it will send HEAD for codes like 204(No Content). I've created a patch for 3.6 to see if this looks reasonable, if review passes, I'll create patches for the rest of the releases. Thanks a lot for po

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Martin Panter added the comment: Sorry to be a pain, but I think the new logic for HEAD is wrong. See review. -- ___ Python tracker ___ __

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Susumu Koshiba added the comment: 3.6 patch(..._05.patch file) -- Added file: http://bugs.python.org/file43225/issue25738_http_reset_content_05.patch ___ Python tracker ___

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Susumu Koshiba added the comment: 3.5 patch. -- Added file: http://bugs.python.org/file43224/issue25738_http_reset_content_3.5_01.patch ___ Python tracker ___ __

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Susumu Koshiba added the comment: Thanks, makes sense to me. I've created patches for 2.7, 3.5, 3.6. 3.5's implementation looked slightly different from 3.6 so I've decided to create a separate patch, just in case. I will upload them 1 by 1 starting with 2.7. It will get a bit noisy here(sorry

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Martin Panter added the comment: I think it would be unusual to have different error handling for GET vs HEAD, given HEAD is supposed to be exactly like GET but without a body. On the other hand, see SimpleHTTPRequestHandler.send_head() for an example of calling send_error(NOT_FOUND) for both

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Susumu Koshiba added the comment: Thanks again for valuable comments. I agree that there could be a case where send_error() gets used for both HEAD and GET, in which case we could send content-length field as an optional field. However, if send_error() is not used for both HEAD and GET, then t

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Martin Panter added the comment: For a HEAD request, I think we should continue to send Content-Length (except in combination with one of the special responses). HEAD is slightly different to 304 Not Modified. With HEAD vs GET, the response code and other header values do not change, but the b

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Susumu Koshiba added the comment: Sorry, re-created a patch to do the right word wrapping and made my comments less verbose. -- Added file: http://bugs.python.org/file43204/issue25738_http_reset_content_04.patch ___ Python tracker

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Susumu Koshiba added the comment: Thanks a lot for a quick review. I've created a new patch with below changes. 1. No content-length and the body to be sent for: a. 204(No Content) b. 205(Reset Content) c. 304(Not Modified) d. response to HEAD request method e. 1xx 2. D

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Martin Panter added the comment: Ah, forget that bit about RESET_CONTENT. If we include Content-Length, it will probably be non-zero, which will be wrong. So your patch is better in that regard. -- ___ Python tracker

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Martin Panter added the comment: I suggest to drop the RESET_CONTENT exception. Content-Length: 0 is explicitly allowed by option a), and is not very different to the general case IMO. Content-Length was added here to help with buggy clients,

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Susumu Koshiba added the comment: Thanks, I've added some comments to the doc and updated the patch. -- Added file: http://bugs.python.org/file43182/issue25738_http_reset_content_02.patch ___ Python tracker _

[issue25738] http.server doesn't handle RESET CONTENT status correctly

R. David Murray added the comment: Well, the reason it makes sense to use send_error, or at least the reason I suspect a lot of people do it, is because then you don't have to call end_headers() yourself. So, acknowleding this, it might be worth mentioning in the docs that the body isn't sent

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Susumu Koshiba added the comment: Patched the behaviors when NO_CONTENT and RESET_CONTENT are sent via send_error(). According to RFCs, they aren't actually errors so sending them through send_response() seems to make the most sense, however. send_error()'s behavior changes in this patch are:

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Mathieu Dupuy added the comment: Oh, my mistake ; I though send_error was to be used internally only, but it's actually a documented public method, that does not enforce to only use "actual" HTTP error code (I wonder what's the point of calling send_error with a non-error status code but that'

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Martin Panter added the comment: Mathieu: What code are you suggesting to delete? The handling of NO_CONTENT etc? That could potentially break people’s code, and I don’t see the harm in leaving it as it is. I would prefer to either bless it in the documentation, or deprecate it if there is a

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Mathieu Dupuy added the comment: I was looking at this issue, and actually the problem is on a different level. The function the patch takes place is "send_errror". As its name suggests, it's only used to send error (I checked in the code : it's only used to send 4XX/5XX reply). I'm sure none o

[issue25738] http.server doesn't handle RESET CONTENT status correctly

Martin Panter added the comment: I left some comments about the tangential changes on Git Hub. Can you write a test case for this? -- nosy: +martin.panter stage: -> patch review type: -> behavior ___ Python tracker

[issue25738] http.server doesn't handle RESET CONTENT status correctly

New submission from SpaceOne: The status 205 RESET CONTENT is not correctly evaluated by http.server. It MUST NOT write a response body to the client. Patch: https://github.com/spaceone/cpython/commit/17048e2e7349cc4861c7fe90299f2c092b8e1604 -- components: Library (Lib) messages: 2554