httplib tunnel infinite loop

[luzakiru]

BPO	13684
Nosy	@terryjreedy, @orsenthil
Files	httplib.patch: httplib.py patch for 2.7.2 consistent with rest of file

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/orsenthil'
closed_at = <Date 2013-02-24.20:47:19.355>
created_at = <Date 2011-12-30.15:56:26.461>
labels = ['library', 'type-crash']
title = 'httplib tunnel infinite loop'
updated_at = <Date 2013-02-24.20:47:19.354>
user = 'https://bugs.python.org/luzakiru'

bugs.python.org fields:

activity = <Date 2013-02-24.20:47:19.354>
actor = 'pitrou'
assignee = 'orsenthil'
closed = True
closed_date = <Date 2013-02-24.20:47:19.355>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2011-12-30.15:56:26.461>
creator = 'luzakiru'
dependencies = []
files = ['24113']
hgrepos = []
issue_num = 13684
keywords = ['patch']
message_count = 6.0
messages = ['150383', '150403', '158551', '159036', '159563', '159564']
nosy_count = 5.0
nosy_names = ['terry.reedy', 'orsenthil', 'rosslagerwall', 'python-dev', 'luzakiru']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue13684'
versions = ['Python 2.7', 'Python 3.2', 'Python 3.3']

[luzakiru]

readline() can return ''. This is handled in most places in httplib but not when a tunnel is used. It leads to a infinite loop that permanently blocks the program while wasting CPU cycles.

For the patch I simply copied the fix that is used elsewhere in the file where readline() is used. It can be fixed in the same way in 2.6.

[terryjreedy]

In 3.2, http.client.py, insertion would be at line 718.
However, only one statement is needed to break. 3.2 elsewhere has
if line in (b'\r\n', b'\n', b''):
break
But I note that at 512, there is the code luzakiru patched in. I think that should perhaps be changed to above also, unless bare \n from reading a server is really impossible.

At 313, i found this misformed code:

         if not line:
            # Presumably, the server closed the connection before
            # sending a valid response.
     raise BadStatusLine(line)

[I am curious -- is it really intended to simply throw away the tunnel server response after the first header?]

[luzakiru]

Although perhaps not optimal, the patch is consistent with the rest of the code and fixes the reasonably severe issue. Could this patch be applied in lieu of a better one that can come later?

[python-dev]

New changeset 790ae45b52be by Senthil Kumaran in branch '2.7':
Fix for bpo-13684 - httplib tunnel infinite loop
http://hg.python.org/cpython/rev/790ae45b52be

New changeset 7787a9aebdc6 by Senthil Kumaran in branch '3.2':
3.2 - Fix for bpo-13684 - httplib tunnel infinite loop
http://hg.python.org/cpython/rev/7787a9aebdc6

New changeset f98fb46ff273 by Senthil Kumaran in branch '2.7':
news for bpo-13684
http://hg.python.org/cpython/rev/f98fb46ff273

New changeset 26631c56d81f by Senthil Kumaran in branch '3.2':
news for bpo-13684
http://hg.python.org/cpython/rev/26631c56d81f

New changeset 1acb252a3858 by Senthil Kumaran in branch 'default':
3.2 - Fix for bpo-13684 - httplib tunnel infinite loop
http://hg.python.org/cpython/rev/1acb252a3858

New changeset 246abd64e830 by Senthil Kumaran in branch 'default':
news for bpo-13684
http://hg.python.org/cpython/rev/246abd64e830

[python-dev]

New changeset 685c1db976c4 by Senthil Kumaran in branch '2.7':
httplib test for early eof response. related to bpo-13684
http://hg.python.org/cpython/rev/685c1db976c4

New changeset afabb0635b15 by Senthil Kumaran in branch '3.2':
httplib test for early eof response. related to bpo-13684
http://hg.python.org/cpython/rev/afabb0635b15

New changeset cfff6a53f4a3 by Senthil Kumaran in branch 'default':
httplib test for early eof response. related to bpo-13684
http://hg.python.org/cpython/rev/cfff6a53f4a3

[orsenthil]

I added a simple test for the early eof condition. It is not specific under _tunnel. I find that Mocks yet to be written that cover the response from httplib ( the mocks in the tests -httplib,urllib2), have their own overridden read() method which may not cover this scenario). The early eof test may be helpful to some extent as general test case. I am closing this bug report as fix has been covered.