classification
Title: Missing "return" in HTTPConnection.send()
Type: Stage: resolved
Components: Versions: Python 3.4, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: asvetlov Nosy List: Evgen.Koval, amaury.forgeotdarc, asvetlov, jeffknupp, orsenthil, python-dev
Priority: normal Keywords: easy, patch

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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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:12ned.deilylinkissue14709 superseder
2014-06-25 19:59:01ned.deilylinkissue12860 superseder
2013-10-27 13:43:41serhiy.storchakalinkissue16904 superseder
2013-04-12 20:00:10asvetlovsetstatus: open -> closed
resolution: fixed
messages: + msg186675

stage: resolved
2013-04-12 19:51:07python-devsetnosy: + python-dev
messages: + msg186673
2013-04-12 14:01:57orsenthilsetassignee: asvetlov
messages: + msg186631
2013-04-11 21:57:29pitrousetnosy: + orsenthil

versions: + Python 3.4, - Python 3.2
2013-04-11 20:07:07asvetlovsetmessages: + msg186586
2013-04-07 11:09:35Evgen.Kovalsetnosy: + asvetlov, Evgen.Koval
messages: + msg186194
2012-12-17 19:43:45jeffknuppsetfiles: + http_client.patch

nosy: + jeffknupp
messages: + msg177658

keywords: + patch
2012-12-10 20:14:32amaury.forgeotdarccreate