changeset: 93679:ba72da4883eb branch: 3.4 parent: 93673:21a7a92f4d0c user: Serhiy Storchaka date: Mon Dec 01 13:07:45 2014 +0200 files: Lib/http/client.py Lib/test/test_httplib.py Misc/NEWS description: Issue #21032. Fixed socket leak if HTTPConnection.getresponse() fails. Original patch by Martin Panter. diff -r 21a7a92f4d0c -r ba72da4883eb Lib/http/client.py --- a/Lib/http/client.py Mon Dec 01 11:06:45 2014 +0200 +++ b/Lib/http/client.py Mon Dec 01 13:07:45 2014 +0200 @@ -1169,18 +1169,22 @@ 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 -r 21a7a92f4d0c -r ba72da4883eb Lib/test/test_httplib.py --- a/Lib/test/test_httplib.py Mon Dec 01 11:06:45 2014 +0200 +++ b/Lib/test/test_httplib.py Mon Dec 01 13:07:45 2014 +0200 @@ -28,6 +28,7 @@ self.fileclass = fileclass self.data = b'' self.sendall_calls = 0 + self.file_closed = False self.host = host self.port = port @@ -38,7 +39,13 @@ def makefile(self, mode, bufsize=None): if mode != 'r' and mode != 'rb': raise client.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 @@ -675,6 +682,22 @@ 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) + class OfflineTest(TestCase): def test_responses(self): self.assertEqual(client.responses[client.NOT_FOUND], "Not Found") diff -r 21a7a92f4d0c -r ba72da4883eb Misc/NEWS --- a/Misc/NEWS Mon Dec 01 11:06:45 2014 +0200 +++ b/Misc/NEWS Mon Dec 01 13:07:45 2014 +0200 @@ -36,6 +36,9 @@ Library ------- +- Issue #21032. Fixed socket leak if HTTPConnection.getresponse() fails. + Original patch by Martin Panter. + - Issue #22960: Add a context argument to xmlrpclib.ServerProxy constructor. - Issue #22915: SAX parser now supports files opened with file descriptor or