homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Missing size argument in readline() method for httplib's class LineAndFileWrapper
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Garfield222, Lukasa, Pierre.Tardy, adambrenecki, berker.peksag, christian.heimes, demian.brecht, icordasc, jitterman, lonetwin, martin.panter, orsenthil, pitrou, python-dev, r.david.murray, raymondr, serhiy.storchaka, stamparm
Priority: normal Keywords: easy, patch

Created on 2013年04月26日 12:27 by stamparm, last changed 2022年04月11日 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
readline.patch Lukasa, 2014年01月17日 19:11 review
readline_2.patch Lukasa, 2015年09月03日 03:15 review
readline_3.patch Lukasa, 2015年09月03日 22:09 review
Messages (24)
msg187841 - (view) Author: Miroslav Stampar (stamparm) Date: 2013年04月26日 12:27
httplib module's auxiliary class LineAndFileWrapper contains a readline() method having no arguments (than self). It's being used in code where commented with "assume it's a Simple-Response from an 0.9 server" as a wrapper for response stream ("self.fp = LineAndFileWrapper(line, self.fp)").
In some cases readline() method requires a size argument (e.g. used with size argument inside HTTPMessage/readheaders(), HTTPResponse/begin(), HTTPResponse/_read_chunked, HTTPConnection/_tunnel, at least in "Python 2.7.1+ (r271:86832, Sep 27 2012, 21:12:17)").
This bug is hard to be reproduced as it requires a little help of "divine intervention".
One such exception looks like (reproduced on one user machine):
...
conn = urllib2.urlopen(req)
File "/usr/lib/python2.7/urllib2.py", line 127, in urlopen
return _opener.open(url, data, timeout)
File "/usr/lib/python2.7/urllib2.py", line 401, in open
response = self._open(req, data)
File "/usr/lib/python2.7/urllib2.py", line 419, in _open
'_open', req)
File "/usr/lib/python2.7/urllib2.py", line 379, in _call_chain
result = func(*args)
File "/usr/share/sqlmap/lib/request/httpshandler.py", line 64, in https_open
return self.do_open(HTTPSConnection if ssl else httplib.HTTPSConnection, req)
File "/usr/lib/python2.7/urllib2.py", line 1178, in do_open
h.request(req.get_method(), req.get_selector(), req.data, headers)
File "/usr/lib/python2.7/httplib.py", line 962, in request
self._send_request(method, url, body, headers)
File "/usr/lib/python2.7/httplib.py", line 996, in _send_request
self.endheaders(body)
File "/usr/lib/python2.7/httplib.py", line 958, in endheaders
self._send_output(message_body)
File "/usr/lib/python2.7/httplib.py", line 818, in _send_output
self.send(msg)
File "/usr/lib/python2.7/httplib.py", line 780, in send
self.connect()
File "/usr/share/sqlmap/lib/request/httpshandler.py", line 46, in connect
sock = create_sock()
File "/usr/share/sqlmap/lib/request/httpshandler.py", line 39, in create_sock
self._tunnel()
File "/usr/lib/python2.7/httplib.py", line 748, in _tunnel
line = response.fp.readline(_MAXLINE + 1)
TypeError: readline() takes exactly 1 argument (2 given)
msg187842 - (view) Author: Miroslav Stampar (stamparm) Date: 2013年04月26日 12:34
Mentioned "divine intervention" could be:
1) Argument strict should be set to 0/False when instantiating HTTPResponse
2) No status should be returned from the backend server (hence the "# assume it's a Simple-Response from an 0.9 server")
3) Any part of response should be read (body, headers, etc.) as HTTPResponse's method begin() uses "self.fp.readline(_MAXLINE + 1)"
msg187850 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013年04月26日 13:40
Thanks for the report. Looks like it is a fairly rare case, which I suppose is why it hasn't been reported before.
If the "could be" turns out to be sufficient, that should be enough for someone to write a test for this. The patch itself should be even easier.
msg187856 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013年04月26日 14:08
Python 3.3 doesn't support HTTP/0.9 and doesn't have LineAndFileWrapper class.
msg188050 - (view) Author: Miroslav Stampar (stamparm) Date: 2013年04月29日 08:41
This trivial "patch" solved the issue (reported back by user):
def _(self, *args):
 return self._readline()
httplib.LineAndFileWrapper._readline = httplib.LineAndFileWrapper.readline
httplib.LineAndFileWrapper.readline = _
msg208333 - (view) Author: Steve (lonetwin) Date: 2014年01月17日 10:59
I just noticed that this is reporducible consistently with the python
requests[1] module, if you route your request through a proxy. I was wondering
whether I should report this as a 'requests' bug or would this be the right
place to add a 'me too' ? Here's the reporducer:
[steve@localhost ~/venvs]$ mkdir requests_bug
[steve@localhost ~/venvs]$ cd requests_bug/
[steve@localhost ~/venvs/requests_bug]$ virtualenv .
New python executable in ./bin/python
Installing Setuptools..............................................................................................................................................................................................................................done.
Installing Pip.....................................................................................................................................................................................................................................................................................................................................done.
[steve@localhost ~/venvs/requests_bug]$ . bin/activate
(requests_bug)[steve@localhost ~/venvs/requests_bug]$ pip install requests
Downloading/unpacking requests
 Downloading requests-2.2.0.tar.gz (421kB): 421kB downloaded
 Running setup.py egg_info for package requests
Installing collected packages: requests
 Running setup.py install for requests
Successfully installed requests
Cleaning up...
(requests_bug)[steve@localhost ~/venvs/requests_bug]$ python
Python 2.7.5 (default, Nov 12 2013, 16:18:42)
[GCC 4.8.2 20131017 (Red Hat 4.8.2-1)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
You've got color, tab completion and pretty-printing. History will be saved
in /home/steve/.pyhistory when you exit.
Typing '\e' will open your $EDITOR with the last executed statement
>>> import requests
>>> requests.get('https://www.google.com/', verify=False)
<Response [200]>
>>> requests.get('https://www.google.com/', verify=False, proxies={'https':'https://192.168.117.157:443'})
Traceback (most recent call last):
 File "<console>", line 1, in <module>
 File "/home/steve/venvs/requests_bug/lib/python2.7/site-packages/requests/api.py", line 55, in get
 return request('get', url, **kwargs)
 File "/home/steve/venvs/requests_bug/lib/python2.7/site-packages/requests/api.py", line 44, in request
 return session.request(method=method, url=url, **kwargs)
 File "/home/steve/venvs/requests_bug/lib/python2.7/site-packages/requests/sessions.py", line 383, in request
 resp = self.send(prep, **send_kwargs)
 File "/home/steve/venvs/requests_bug/lib/python2.7/site-packages/requests/sessions.py", line 486, in send
 r = adapter.send(request, **kwargs)
 File "/home/steve/venvs/requests_bug/lib/python2.7/site-packages/requests/adapters.py", line 334, in send
 timeout=timeout
 File "/home/steve/venvs/requests_bug/lib/python2.7/site-packages/requests/packages/urllib3/connectionpool.py", line 475, in urlopen
 conn = self._get_conn(timeout=pool_timeout)
 File "/home/steve/venvs/requests_bug/lib/python2.7/site-packages/requests/packages/urllib3/connectionpool.py", line 217, in _get_conn
 return conn or self._new_conn()
 File "/home/steve/venvs/requests_bug/lib/python2.7/site-packages/requests/packages/urllib3/connectionpool.py", line 656, in _new_conn
 return self._prepare_conn(conn)
 File "/home/steve/venvs/requests_bug/lib/python2.7/site-packages/requests/packages/urllib3/connectionpool.py", line 625, in _prepare_conn
 conn.connect()
 File "/home/steve/venvs/requests_bug/lib/python2.7/site-packages/requests/packages/urllib3/connection.py", line 90, in connect
 self._tunnel()
 File "/usr/lib64/python2.7/httplib.py", line 759, in _tunnel
 line = response.fp.readline(_MAXLINE + 1)
TypeError: readline() takes exactly 1 argument (2 given)
>>>
cheers,
- steve
[1] requests-2.2.0
msg208341 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014年01月17日 15:27
Requests may well want to put a workaround in place. To fix it in 2.7, we need a patch that actually respects the size argument, and a unit test.
msg208352 - (view) Author: Cory Benfield (Lukasa) * Date: 2014年01月17日 18:42
I've been messing around trying to write a test for this, and it looks like genuinely the only place this can be hit is in _tunnel(). None of the other readline() calls can be hit with the LineAndFileWrapper() wrapping the file descriptor.
msg208354 - (view) Author: Cory Benfield (Lukasa) * Date: 2014年01月17日 19:11
Alright, here's a patch.
I'm not ecstatic about this test: it's utterly contrived, but it also seems to be the least bad way to reproduce this bug. Let me know if you strongly object and I'll take another swing at it (it'll probably involve extending the FakeSocket quite extensively).
msg219858 - (view) Author: Pierre Tardy (Pierre.Tardy) * Date: 2014年06月05日 23:50
I made a similar patch today to fix the same issue, and I confirm the problem and the correctness of the solution
Please approve the patch and fix this bug.
msg227734 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014年09月27日 21:18
I tried the patch, but the test fails for me:
test test_httplib failed -- Traceback (most recent call last):
 File "/home/rdmurray/python/p27/Lib/test/test_httplib.py", line 434, in test_proxy_tunnel_without_status_line
 conn.set_tunnel('foo')
 File "/home/rdmurray/python/p27/Lib/httplib.py", line 734, in set_tunnel
 raise RuntimeError("Can't setup tunnel for established connection.")
RuntimeError: Can't setup tunnel for established connection.
msg243010 - (view) Author: JitterMan (jitterman) Date: 2015年05月12日 20:29
I ran into this problem when I gave https_proxy an invalid value:
 export https_proxy=http://foo.com
No divine intervention required. I was just trying to determine what message was printed with the proxy environment variable was set incorrectly.
Perhaps that will help you develop a more reasonable testcase.
msg249512 - (view) Author: Garfield222 (Garfield222) Date: 2015年09月01日 19:46
Same error happens if http_proxy="223.19.230.181:80" variable is set.
msg249513 - (view) Author: Garfield222 (Garfield222) Date: 2015年09月01日 19:51
This error occurs when using youtube-dl 
Complete log is here:
https://github.com/rg3/youtube-dl/issues/6727 
msg249523 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015年09月02日 01:27
Left some review comments. But I don’t completely understand the test case. Is it true there are proxies that do not send a HTTP status line (like HTTP/1.0 200 Connection Established), but do sned the rest of the HTTP header section? This is what the test case seems to be modelling:
Client to proxy request:
b"CONNECT foo:80 HTTP/1.0\r\n"
b"\r\n"
Proxy (plus tunnelled?) response, minus a status line:
b"X-Foo: bar\r\n"
b"\r\n"
b"hello world"
It seems more likely that the proxies related to this bug are misbehaving, or there is some other network problem. Of the proxies mentioned in this report that are accessible to me, both of them do not appear to accept CONNECT requests at all. As soon as the first CONNECT line is send, a HTML blob saying "400 Bad Request" is received, without any HTTP header at all.
msg249524 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015年09月02日 01:33
A test case with data like this would make more sense to me:
# Response without HTTP status line or header, modelling responses sent by
# some proxies that do not support CONNECT
body = b"<html>400 Bad Request</html>"
msg249588 - (view) Author: Cory Benfield (Lukasa) * Date: 2015年09月03日 02:03
Martin: as noted in the comments on this issue, I believed the specific test case did not match any of the problems people were encountering: it was just the least bad way to trigger the specific flow that was being encountered.
In practice for these issues it's likely some other error condition is involved. Regardless, Python 2.7 claims to support HTTP/0.9, which certainly allows for proxies to not send a status line, so the test case is a reasonable example of what should be supported by httplib.
I'll take another run at this patch to see if I can clean it up.
msg249598 - (view) Author: Cory Benfield (Lukasa) * Date: 2015年09月03日 03:15
Ok, new patch now attached.
msg249603 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015年09月03日 04:40
I still don’t think the test case is a reasonable example. According to <http://www.w3.org/Protocols/HTTP/AsImplemented.html>, HTTP 0.9 only supported GET, not CONNECT, and doesn’t include any header fields in the protocol (such as your X-Foo: bar). It would make more sense to me to adjust the check at <https://hg.python.org/cpython/file/69ea73015132/Lib/httplib.py#l813> to also check for version == "HTTP/0.9". That would probably eliminate the need to mess too much with the details of the LineAndFileWrapper class.
But if you wanted to parse the CONNECT response as a HTTP 0.9 response, I think you should not try to parse any header, because that will tend to skip over the start of the actual response.
Also, I assume the change to "configure" is unrelated.
msg249695 - (view) Author: Cory Benfield (Lukasa) * Date: 2015年09月03日 21:57
Martin, the idea of simply rejecting the LifeAndFileWrapper HTTP/0.9 fallback for _tunnel feels reasonable to me, that can work. Let me whip up an alternative patch that does that.
msg249698 - (view) Author: Cory Benfield (Lukasa) * Date: 2015年09月03日 22:09
Ok, version three of the patch is now available.
msg249752 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015年09月04日 10:55
Patch 3 looks better IMO. Now instead of the TypeError, we should see an error something like
socket.error: Tunnel connection failed: 200
I’ll commit this in a day or so, perhaps with an improved error message, like "Invalid response from tunnel request".
msg250032 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015年09月07日 01:20
New changeset 3510e5d435db by Martin Panter <vadmium> in branch '2.7':
Issue #17849: Raise sensible exception for invalid HTTP tunnel response
https://hg.python.org/cpython/rev/3510e5d435db 
msg250033 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015年09月07日 01:23
Thanks Cory. Error now looks like this:
$ https_proxy=http://223.19.230.181:80 ./python -c 'from urllib2 import *; urlopen("https://bugs.python.org/issue17849")'
[. . .]
urllib2.URLError: <urlopen error Invalid response from tunnel request>
History
Date User Action Args
2022年04月11日 14:57:44adminsetgithub: 62049
2015年09月07日 01:23:30martin.pantersetstatus: open -> closed
resolution: fixed
messages: + msg250033

stage: patch review -> resolved
2015年09月07日 01:20:45python-devsetnosy: + python-dev
messages: + msg250032
2015年09月04日 10:55:59martin.pantersetmessages: + msg249752
2015年09月03日 22:09:37Lukasasetfiles: + readline_3.patch

messages: + msg249698
2015年09月03日 21:57:17Lukasasetmessages: + msg249695
2015年09月03日 04:40:22martin.pantersetmessages: + msg249603
2015年09月03日 03:15:08Lukasasetfiles: + readline_2.patch

messages: + msg249598
2015年09月03日 02:03:47Lukasasetmessages: + msg249588
2015年09月02日 01:33:26martin.pantersetmessages: + msg249524
2015年09月02日 01:27:07martin.pantersetnosy: + martin.panter
messages: + msg249523
2015年09月01日 19:51:47Garfield222setmessages: + msg249513
2015年09月01日 19:46:37Garfield222setnosy: + Garfield222
messages: + msg249512
2015年05月31日 03:32:17martin.panterlinkissue15928 superseder
2015年05月13日 07:13:11berker.peksagsetnosy: + berker.peksag

stage: needs patch -> patch review
2015年05月12日 20:29:56jittermansetmessages: + msg243010
2015年05月12日 20:12:58demian.brechtsetnosy: + demian.brecht
2015年05月12日 20:06:29r.david.murraysetnosy: + jitterman
2015年05月12日 20:06:04r.david.murraylinkissue24171 superseder
2014年09月27日 21:18:11r.david.murraysetmessages: + msg227734
stage: commit review -> needs patch
2014年06月30日 13:35:02r.david.murraysetstage: needs patch -> commit review
2014年06月30日 13:11:31icordascsetnosy: + icordasc
2014年06月05日 23:50:32Pierre.Tardysetnosy: + Pierre.Tardy
messages: + msg219858
2014年01月17日 19:11:42Lukasasetfiles: + readline.patch
keywords: + patch
messages: + msg208354
2014年01月17日 18:42:43Lukasasetnosy: + Lukasa
messages: + msg208352
2014年01月17日 15:27:45r.david.murraysetmessages: + msg208341
2014年01月17日 10:59:36lonetwinsetnosy: + lonetwin
messages: + msg208333
2013年11月15日 22:40:02raymondrsetnosy: + raymondr
2013年05月03日 20:07:59serhiy.storchakasetnosy: + orsenthil, pitrou
2013年05月03日 04:38:55adambreneckisetnosy: + adambrenecki
2013年04月29日 08:41:12stamparmsetmessages: + msg188050
2013年04月26日 14:08:32serhiy.storchakasetnosy: + christian.heimes, serhiy.storchaka

messages: + msg187856
versions: - Python 3.3, Python 3.4
2013年04月26日 13:40:40r.david.murraysettype: behavior
versions: - Python 2.6, Python 3.1, Python 3.2, Python 3.5
keywords: + easy
nosy: + r.david.murray

messages: + msg187850
stage: needs patch
2013年04月26日 12:34:48stamparmsetmessages: + msg187842
2013年04月26日 12:27:04stamparmcreate

AltStyle によって変換されたページ (->オリジナル) /