Issue #21032. Fixed socket leak if HTTPConnection.getresponse() fails.

Original patch by Martin Panter.

3 files changed, 38 insertions, 11 deletions

diff --git a/Lib/http/client.py b/Lib/http/client.py
index 31d066b..4a8b4ee 100644
--- a/Lib/http/client.py
+++ b/Lib/http/client.py
@@ -1233,18 +1233,22 @@ class HTTPConnection:
         else:
             response = self.response_class(self.sock, method=self._method)
 
-        response.begin()
-        assert response.will_close != _UNKNOWN
-        self.__state = _CS_IDLE
+        try:
+            response.begin()
+            assert response.will_close != _UNKNOWN
+            self.__state = _CS_IDLE
 
-        if response.will_close:
-            # this effectively passes the connection to the response
-            self.close()
-        else:
-            # remember this, so we can tell when it is complete
-            self.__response = response
+            if response.will_close:
+                # this effectively passes the connection to the response
+                self.close()
+            else:
+                # remember this, so we can tell when it is complete
+                self.__response = response
 
-        return response
+            return response
+        except:
+            response.close()
+            raise
 
 try:
     import ssl
diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
index e693f8c..ddeabe1 100644
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@ -48,6 +48,7 @@ class FakeSocket:
         self.fileclass = fileclass
         self.data = b''
         self.sendall_calls = 0
+        self.file_closed = False
         self.host = host
         self.port = port
 
@@ -60,9 +61,12 @@ class FakeSocket:
             raise client.UnimplementedFileMode()
         # keep the file around so we can check how much was read from it
         self.file = self.fileclass(self.text)
-        self.file.close = lambda:None #nerf close ()
+        self.file.close = self.file_close #nerf close ()
         return self.file
 
+    def file_close(self):
+        self.file_closed = True
+
     def close(self):
         pass
 
@@ -676,6 +680,22 @@ class BasicTest(TestCase):
         conn.request('POST', '/', body)
         self.assertGreater(sock.sendall_calls, 1)
 
+    def test_error_leak(self):
+        # Test that the socket is not leaked if getresponse() fails
+        conn = client.HTTPConnection('example.com')
+        response = None
+        class Response(client.HTTPResponse):
+            def __init__(self, *pos, **kw):
+                nonlocal response
+                response = self  # Avoid garbage collector closing the socket
+                client.HTTPResponse.__init__(self, *pos, **kw)
+        conn.response_class = Response
+        conn.sock = FakeSocket('')  # Emulate server dropping connection
+        conn.request('GET', '/')
+        self.assertRaises(client.BadStatusLine, conn.getresponse)
+        self.assertTrue(response.closed)
+        self.assertTrue(conn.sock.file_closed)
+
     def test_chunked_extension(self):
         extra = '3;foo=bar\r\n' + 'abc\r\n'
         expected = chunked_expected + b'abc'
diff --git a/Misc/NEWS b/Misc/NEWS
index 8b30dd1..72d863f 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -191,6 +191,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #21032. Fixed socket leak if HTTPConnection.getresponse() fails.
+  Original patch by Martin Panter.
+
 - Issue #22407: Deprecated the use of re.LOCALE flag with str patterns or
   re.ASCII. It was newer worked.