diff options
| -rw-r--r-- | Lib/test/test_urllib.py | 59 | ||||
| -rw-r--r-- | Lib/test/test_urllib2.py | 29 | ||||
| -rw-r--r-- | Lib/urllib/request.py | 25 | ||||
| -rw-r--r-- | Misc/ACKS | 1 | ||||
| -rw-r--r-- | Misc/NEWS | 3 |
5 files changed, 79 insertions, 38 deletions
diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py index 1a5013e..7eb34c8 100644 --- a/Lib/test/test_urllib.py +++ b/Lib/test/test_urllib.py @@ -48,43 +48,48 @@ def urlopen(url, data=None, proxies=None): return opener.open(url, data) -class FakeHTTPMixin(object): - def fakehttp(self, fakedata): - class FakeSocket(io.BytesIO): - io_refs = 1 +def fakehttp(fakedata): + class FakeSocket(io.BytesIO): + io_refs = 1 - def sendall(self, data): - FakeHTTPConnection.buf = data + def sendall(self, data): + FakeHTTPConnection.buf = data - def makefile(self, *args, **kwds): - self.io_refs += 1 - return self + 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 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 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) + def close(self): + self.io_refs -= 1 + if self.io_refs == 0: + io.BytesIO.close(self) + + class FakeHTTPConnection(http.client.HTTPConnection): - class FakeHTTPConnection(http.client.HTTPConnection): + # buffer to store data for verification in urlopen tests. + buf = None + fakesock = FakeSocket(fakedata) - # buffer to store data for verification in urlopen tests. - buf = None + def connect(self): + self.sock = self.fakesock - def connect(self): - self.sock = FakeSocket(fakedata) + return FakeHTTPConnection + +class FakeHTTPMixin(object): + def fakehttp(self, 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 --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py index 7620876..9ea39a4 100644 --- a/Lib/test/test_urllib2.py +++ b/Lib/test/test_urllib2.py @@ -1,5 +1,6 @@ import unittest from test import support +from test import test_urllib import os import io @@ -13,6 +14,7 @@ import urllib.request 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 @@ class HandlerTests(unittest.TestCase): 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 --git a/Lib/urllib/request.py b/Lib/urllib/request.py index a17c868..67c7566 100644 --- a/Lib/urllib/request.py +++ b/Lib/urllib/request.py @@ -1170,18 +1170,21 @@ class AbstractHTTPHandler(BaseHandler): 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 @@ -1003,6 +1003,7 @@ Mike Pall Todd R. Palmer Juan David Ibáñez Palomar Jan Palus +Martin Panter Mathias Panzenböck M. Papillon Peter Parente @@ -32,6 +32,9 @@ Core and Builtins 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. |