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

Original patch by Martin Panter.

diff --git a/Lib/httplib.py b/Lib/httplib.py
index 7c2790685858dd6c380ebea7d54587b9bcaaa4ab..1224989d015253f599a92ec6705be1060cadb79c 100644 (file)
--- a/Lib/httplib.py
+++ b/Lib/httplib.py
@@ -1070,18 +1070,22 @@ class HTTPConnection:
             kwds["buffering"] = True;
         response = self.response_class(*args, **kwds)
 
-        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
 
 
 class HTTP:

diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
index 3e7a57e7f09b9fd1253c15452f487d45753dd26c..a73a28be62af59e320d54394adef829ff182e29a 100644 (file)
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@ -25,6 +25,7 @@ class FakeSocket:
         self.text = text
         self.fileclass = fileclass
         self.data = ''
+        self.file_closed = False
         self.host = host
         self.port = port
 
@@ -34,7 +35,13 @@ class FakeSocket:
     def makefile(self, mode, bufsize=None):
         if mode != 'r' and mode != 'rb':
             raise httplib.UnimplementedFileMode()
-        return self.fileclass(self.text)
+        # keep the file around so we can check how much was read from it
+        self.file = self.fileclass(self.text)
+        self.file.close = self.file_close #nerf close ()
+        return self.file
+
+    def file_close(self):
+        self.file_closed = True
 
     def close(self):
         pass
@@ -433,6 +440,22 @@ class BasicTest(TestCase):
         self.assertEqual(resp.read(), '')
         self.assertTrue(resp.isclosed())
 
+    def test_error_leak(self):
+        # Test that the socket is not leaked if getresponse() fails
+        conn = httplib.HTTPConnection('example.com')
+        response = []
+        class Response(httplib.HTTPResponse):
+            def __init__(self, *pos, **kw):
+                response.append(self)  # Avoid garbage collector closing the socket
+                httplib.HTTPResponse.__init__(self, *pos, **kw)
+        conn.response_class = Response
+        conn.sock = FakeSocket('')  # Emulate server dropping connection
+        conn.request('GET', '/')
+        self.assertRaises(httplib.BadStatusLine, conn.getresponse)
+        self.assertTrue(response)
+        #self.assertTrue(response[0].closed)
+        self.assertTrue(conn.sock.file_closed)
+
 class OfflineTest(TestCase):
     def test_responses(self):
         self.assertEqual(httplib.responses[httplib.NOT_FOUND], "Not Found")

diff --git a/Misc/ACKS b/Misc/ACKS
index d44240f0f9d90d23ee139943da15e818a30e7449..5372ee7e5b27394d99a80be923b2d8216ed9637a 100644 (file)
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -1010,6 +1010,7 @@ Todd R. Palmer
 Juan David Ibáñez Palomar
 Jan Palus
 Yongzhi Pan
+Martin Panter
 Mathias Panzenböck
 M. Papillon
 Peter Parente

diff --git a/Misc/NEWS b/Misc/NEWS
index 4b42d627d75951fee3059b15607de4712e5b7569..b9835a545003c73ef1474df2ac289b55d5d76bcc 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -13,6 +13,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #21032. Fixed socket leak if HTTPConnection.getresponse() fails.
+  Original patch by Martin Panter.
+
 - Issue #22609: Constructors and update methods of mapping classes in the
   collections module now accept the self keyword argument.