changeset: 92366:c1fb19907cc4 branch: 3.4 parent: 92363:e8447da8791d user: Serhiy Storchaka date: Sat Sep 06 21:41:39 2014 +0300 files: Lib/test/test_urllib.py Lib/test/test_urllib2.py Lib/urllib/request.py Misc/ACKS Misc/NEWS description: Issue #19524: Fixed resource leak in the HTTP connection when an invalid response is received. Patch by Martin Panter. diff -r e8447da8791d -r c1fb19907cc4 Lib/test/test_urllib.py --- a/Lib/test/test_urllib.py Sat Sep 06 20:38:23 2014 +1000 +++ b/Lib/test/test_urllib.py Sat Sep 06 21:41:39 2014 +0300 @@ -48,43 +48,48 @@ return opener.open(url, data) +def fakehttp(fakedata): + class FakeSocket(io.BytesIO): + io_refs = 1 + + def sendall(self, data): + FakeHTTPConnection.buf = data + + def makefile(self, *args, **kwds): + self.io_refs += 1 + return self + + def read(self, amt=None): + if self.closed: + return b"" + return io.BytesIO.read(self, amt) + + def readline(self, length=None): + if self.closed: + return b"" + return io.BytesIO.readline(self, length) + + def close(self): + self.io_refs -= 1 + if self.io_refs == 0: + io.BytesIO.close(self) + + class FakeHTTPConnection(http.client.HTTPConnection): + + # buffer to store data for verification in urlopen tests. + buf = None + fakesock = FakeSocket(fakedata) + + def connect(self): + self.sock = self.fakesock + + return FakeHTTPConnection + + class FakeHTTPMixin(object): def fakehttp(self, fakedata): - class FakeSocket(io.BytesIO): - io_refs = 1 - - def sendall(self, data): - FakeHTTPConnection.buf = data - - def makefile(self, *args, **kwds): - self.io_refs += 1 - return self - - def read(self, amt=None): - if self.closed: - return b"" - return io.BytesIO.read(self, amt) - - def readline(self, length=None): - if self.closed: - return b"" - return io.BytesIO.readline(self, length) - - def close(self): - self.io_refs -= 1 - if self.io_refs == 0: - io.BytesIO.close(self) - - class FakeHTTPConnection(http.client.HTTPConnection): - - # buffer to store data for verification in urlopen tests. - buf = None - - def connect(self): - self.sock = FakeSocket(fakedata) - self._connection_class = http.client.HTTPConnection - http.client.HTTPConnection = FakeHTTPConnection + http.client.HTTPConnection = fakehttp(fakedata) def unfakehttp(self): http.client.HTTPConnection = self._connection_class diff -r e8447da8791d -r c1fb19907cc4 Lib/test/test_urllib2.py --- a/Lib/test/test_urllib2.py Sat Sep 06 20:38:23 2014 +1000 +++ b/Lib/test/test_urllib2.py Sat Sep 06 21:41:39 2014 +0300 @@ -1,5 +1,6 @@ import unittest from test import support +from test import test_urllib import os import io @@ -13,6 +14,7 @@ from urllib.request import Request, OpenerDirector, _parse_proxy, _proxy_bypass_macosx_sysconf from urllib.parse import urlparse import urllib.error +import http.client # XXX # Request @@ -1393,6 +1395,33 @@ self.assertEqual(len(http_handler.requests), 1) self.assertFalse(http_handler.requests[0].has_header(auth_header)) + def test_http_closed(self): + """Test the connection is cleaned up when the response is closed""" + for (transfer, data) in ( + ("Connection: close", b"data"), + ("Transfer-Encoding: chunked", b"4\r\ndata\r\n0\r\n\r\n"), + ("Content-Length: 4", b"data"), + ): + header = "HTTP/1.1 200 OK\r\n{}\r\n\r\n".format(transfer) + conn = test_urllib.fakehttp(header.encode() + data) + handler = urllib.request.AbstractHTTPHandler() + req = Request("http://dummy/") + req.timeout = None + with handler.do_open(conn, req) as resp: + resp.read() + self.assertTrue(conn.fakesock.closed, + "Connection not closed with {!r}".format(transfer)) + + def test_invalid_closed(self): + """Test the connection is cleaned up after an invalid response""" + conn = test_urllib.fakehttp(b"") + handler = urllib.request.AbstractHTTPHandler() + req = Request("http://dummy/") + req.timeout = None + with self.assertRaises(http.client.BadStatusLine): + handler.do_open(conn, req) + self.assertTrue(conn.fakesock.closed, "Connection not closed") + class MiscTests(unittest.TestCase): diff -r e8447da8791d -r c1fb19907cc4 Lib/urllib/request.py --- a/Lib/urllib/request.py Sat Sep 06 20:38:23 2014 +1000 +++ b/Lib/urllib/request.py Sat Sep 06 21:41:39 2014 +0300 @@ -1170,18 +1170,21 @@ h.set_tunnel(req._tunnel_host, headers=tunnel_headers) try: - h.request(req.get_method(), req.selector, req.data, headers) - except OSError as err: # timeout error - h.close() - raise URLError(err) - else: + try: + h.request(req.get_method(), req.selector, req.data, headers) + except OSError as err: # timeout error + raise URLError(err) r = h.getresponse() - # If the server does not send us a 'Connection: close' header, - # HTTPConnection assumes the socket should be left open. Manually - # mark the socket to be closed when this response object goes away. - if h.sock: - h.sock.close() - h.sock = None + except: + h.close() + raise + + # If the server does not send us a 'Connection: close' header, + # HTTPConnection assumes the socket should be left open. Manually + # mark the socket to be closed when this response object goes away. + if h.sock: + h.sock.close() + h.sock = None r.url = req.get_full_url() # This line replaces the .msg attribute of the HTTPResponse diff -r e8447da8791d -r c1fb19907cc4 Misc/ACKS --- a/Misc/ACKS Sat Sep 06 20:38:23 2014 +1000 +++ b/Misc/ACKS Sat Sep 06 21:41:39 2014 +0300 @@ -1003,6 +1003,7 @@ Todd R. Palmer Juan David Ibáñez Palomar Jan Palus +Martin Panter Mathias Panzenböck M. Papillon Peter Parente diff -r e8447da8791d -r c1fb19907cc4 Misc/NEWS --- a/Misc/NEWS Sat Sep 06 20:38:23 2014 +1000 +++ b/Misc/NEWS Sat Sep 06 21:41:39 2014 +0300 @@ -32,6 +32,9 @@ Library ------- +- Issue #19524: Fixed resource leak in the HTTP connection when an invalid + response is received. Patch by Martin Panter. + - Issue #22051: turtledemo no longer reloads examples to re-run them. Initialization of variables and gui setup should be done in main(), which is called each time a demo is run, but not on import.