Created on 2012-12-10 20:14 by amaury.forgeotdarc, last changed 2013-04-12 20:00 by asvetlov. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| http_client.patch | jeffknupp, 2012-12-17 19:43 | review | ||
| Messages (7) | |||
|---|---|---|---|
| msg177312 - (view) | Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *  |
Date: 2012-12-10 20:14 | |
HTTPConnection.send() accepts a bytes string, a file, and any iterable. When a file is passed, data is read in blocks until read() returns an empty string. But because a "return" statement is missing, execution continues with an attempt to iterate the file again... This exits quickly most of the time, but this can lead to surprising behavior if more data is available, or for custom implementations of the file object. |
|||
| msg177658 - (view) | Author: Jeff Knupp (jeffknupp) * | Date: 2012-12-17 19:43 | |
I'm assuming this is the patch you were looking for. However, there are a couple of unrelated issues with http.client.send that jumped out at me: 1. Encoding a file handed directly to send() seems wrong. If a client wants to send a file encoded using something other than iso-8859-1, we've effectively short-circuited that. Since the normal request() calls take care of encoding, it seems send() should be for those that 'know what they're doing'. Also, nowhere in the send() documentation does it state that send() will perform this encoding (and only on a file, but not on a string?). Removing the burden of encoding if a file-like object is passed seems more reasonable and would make the code considerably clearer. If anyone agrees, I'll open a new ticket with patch for this issue. |
|||
| msg186194 - (view) | Author: Evgen Koval (Evgen.Koval) * | Date: 2013-04-07 11:09 | |
I reviewed and verified this patch, and it looks correct to me. |
|||
| msg186586 - (view) | Author: Andrew Svetlov (asvetlov) * |
Date: 2013-04-11 20:07 | |
LGTM, will commit if no objects. Let's leave encoding problems for upcoming issue. Please file new one if needed. |
|||
| msg186631 - (view) | Author: Senthil Kumaran (orsenthil) * |
Date: 2013-04-12 14:01 | |
Andrew - Please go ahead with commit. I reviewed the patch and it looks good to me as well. It is a bug so it should be backported. Thanks for the patch and a good test, Jeff Knupp. Agree with your reasoning / lack of documentation on send behavior w.r.t to encoding, Please open a new ticket for that. |
|||
| msg186673 - (view) | Author: Roundup Robot (python-dev) | Date: 2013-04-12 19:51 | |
New changeset 70c7687de1cd by Andrew Svetlov in branch '3.3': Issue #16658: add missing return to HTTPConnection.send(). http://hg.python.org/cpython/rev/70c7687de1cd New changeset 2b89e9a6b482 by Andrew Svetlov in branch 'default': Issue #16658: add missing return to HTTPConnection.send(). http://hg.python.org/cpython/rev/2b89e9a6b482 |
|||
| msg186675 - (view) | Author: Andrew Svetlov (asvetlov) * |
Date: 2013-04-12 20:00 | |
Pushed for 3.3 and 3.4. As I see 2.7 has no this problem. Close the issue. Thanks for all. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2014-07-04 00:45:12 | ned.deily | link | issue14709 superseder |
| 2014-06-25 19:59:01 | ned.deily | link | issue12860 superseder |
| 2013-10-27 13:43:41 | serhiy.storchaka | link | issue16904 superseder |
| 2013-04-12 20:00:10 | asvetlov | set | status: open -> closed resolution: fixed messages: + msg186675 stage: resolved |
| 2013-04-12 19:51:07 | python-dev | set | nosy:
+ python-dev messages: + msg186673 |
| 2013-04-12 14:01:57 | orsenthil | set | assignee: asvetlov messages: + msg186631 |
| 2013-04-11 21:57:29 | pitrou | set | nosy:
+ orsenthil versions: + Python 3.4, - Python 3.2 |
| 2013-04-11 20:07:07 | asvetlov | set | messages: + msg186586 |
| 2013-04-07 11:09:35 | Evgen.Koval | set | nosy:
+ asvetlov, Evgen.Koval messages: + msg186194 |
| 2012-12-17 19:43:45 | jeffknupp | set | files:
+ http_client.patch nosy: + jeffknupp messages: + msg177658 keywords: + patch |
| 2012-12-10 20:14:32 | amaury.forgeotdarc | create | |