From b491e0521fd6e1a2e6f3e6354a2a296c7c8a2915 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 1 Dec 2014 13:07:45 +0200 Subject: Issue #21032. Fixed socket leak if HTTPConnection.getresponse() fails. Original patch by Martin Panter. --- Lib/http/client.py | 24 ++++++++++++++---------- Lib/test/test_httplib.py | 25 ++++++++++++++++++++++++- Misc/NEWS | 3 +++ 3 files changed, 41 insertions(+), 11 deletions(-) diff --git a/Lib/http/client.py b/Lib/http/client.py index cb1a535..281e7f2 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -1169,18 +1169,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 3b57d09..933e5c4 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -28,6 +28,7 @@ class FakeSocket: self.fileclass = fileclass self.data = b'' self.sendall_calls = 0 + self.file_closed = False self.host = host self.port = port @@ -38,7 +39,13 @@ class FakeSocket: 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 @@ 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) + class OfflineTest(TestCase): def test_responses(self): self.assertEqual(client.responses[client.NOT_FOUND], "Not Found") diff --git a/Misc/NEWS b/Misc/NEWS index 8089fac..9cac209 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -36,6 +36,9 @@ Core and Builtins 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 -- cgit v0.12