Issue #19524: Fixed resource leak in the HTTP connection when an invalid

response is received.  Patch by Martin Panter.

Lib/test/test_urllib.py

@@ -48,8 +48,7 @@ def urlopen(url, data=None, proxies=None):
         return opener.open(url, data)
 
 
-class FakeHTTPMixin(object):
-    def fakehttp(self, fakedata):
+def fakehttp(fakedata):
     class FakeSocket(io.BytesIO):
         io_refs = 1
 
@@ -79,12 +78,18 @@ class FakeHTTPMixin(object):
 
         # buffer to store data for verification in urlopen tests.
         buf = None
+        fakesock = FakeSocket(fakedata)
 
         def connect(self):
-                self.sock = FakeSocket(fakedata)
+            self.sock = self.fakesock
+
+    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

Lib/test/test_urllib2.py

 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):
 

Lib/urllib/request.py

@@ -1169,13 +1169,16 @@ class AbstractHTTPHandler(BaseHandler):
                 del headers[proxy_auth_hdr]
             h.set_tunnel(req._tunnel_host, headers=tunnel_headers)
 
+        try:
             try:
                 h.request(req.get_method(), req.selector, req.data, headers)
             except OSError as err: # timeout error
-            h.close()
                 raise URLError(err)
-        else:
             r = h.getresponse()
+        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.

Misc/ACKS

@@ -1012,6 +1012,7 @@ Mike Pall
 Todd R. Palmer
 Juan David Ibáñez Palomar
 Jan Palus
+Martin Panter
 Mathias Panzenböck
 M. Papillon
 Peter Parente

Misc/NEWS

@@ -132,6 +132,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 #20421: Add a .version() method to SSL sockets exposing the actual
   protocol version in use.