# HG changeset patch # Parent eaa38b75cc7812dbf079801657f6eae9ebb85157 Issue 3566: Improve support for persistent connections * Add http.client.ConnectionClosed exception * HTTPConnection.close() implicitly called * BadStatusLine or ConnectionError still raised on first getresponse() * request() ConnectionError does not imply server did not send a response * ConnectionClosed wraps ECONNRESET from first recv() of status line diff -r eaa38b75cc78 Doc/library/http.client.rst --- a/Doc/library/http.client.rst Fri Jan 23 17:30:26 2015 -0500 +++ b/Doc/library/http.client.rst Sun Jan 25 00:37:09 2015 +0000 @@ -93,7 +93,7 @@ parameter. -.. class:: HTTPResponse(sock, debuglevel=0, method=None, url=None) +.. class:: HTTPResponse(sock, debuglevel=0, method=None, url=None, connection_reused=False) Class whose instances are returned upon successful connection. Not instantiated directly by user. @@ -102,6 +102,9 @@ The *strict* parameter was removed. HTTP 0.9 style "Simple Responses" are no longer supported. + .. versionadded:: 3.5 + The *connection_reused* parameter. + The following exceptions are raised as appropriate: @@ -166,7 +169,18 @@ .. exception:: BadStatusLine A subclass of :exc:`HTTPException`. Raised if a server responds with a HTTP - status code that we don't understand. + status code that we don't understand, or the server signals the end of + the stream when a response is expected. + + +.. exception:: ConnectionClosed + + Raised by :meth:`HTTPConnection.getresponse` if a persistent connection + is closed. + + .. versionadded:: 3.5 + Previously, :exc:`BadStatusLine` or :exc:`ConnectionError` was raised. + This is a subclass of :exc:`BadStatusLine` for backwards compatibility. The constants defined in this module are: @@ -226,11 +240,23 @@ Should be called after a request is sent to get the response from the server. Returns an :class:`HTTPResponse` instance. + If this is the second or subsequent request of a persistent connection + (after it last reconnected), and the connection was closed without + any response received, :exc:`ConnectionClosed` is raised. However + if this is the first request of a connection, :exc:`BadStatusLine` is + always raised. If a persistent connection was closed, the + :class:`HTTPConnection` object will be ready to reconnect when + a new request is sent. + .. note:: Note that you must have read the whole response before you can send a new request to the server. + .. versionchanged:: 3.5 + The :meth:`close` method no longer needs to be called to reconnect + after a persistent connection is dropped. + .. method:: HTTPConnection.set_debuglevel(level) @@ -269,7 +295,9 @@ .. method:: HTTPConnection.connect() - Connect to the server specified when the object was created. + Connect to the server specified when the object was created. Normally + called automatically when making a request and the client is + not connected. .. method:: HTTPConnection.close() diff -r eaa38b75cc78 Lib/http/client.py --- a/Lib/http/client.py Fri Jan 23 17:30:26 2015 -0500 +++ b/Lib/http/client.py Sun Jan 25 00:37:09 2015 +0000 @@ -171,7 +171,8 @@ # text following RFC 2047. The basic status line parsing only # accepts iso-8859-1. - def __init__(self, sock, debuglevel=0, method=None, url=None): + def __init__(self, sock, debuglevel=0, method=None, url=None, + connection_reused=False): # If the response includes a content-length header, we need to # make sure that the client doesn't read more than the # specified number of bytes. If it does, it will block until @@ -182,6 +183,7 @@ self.fp = sock.makefile("rb") self.debuglevel = debuglevel self._method = method + self._connection_reused = connection_reused # The HTTPResponse object is returned via urllib. The clients # of http and urllib expect different attributes for the @@ -238,6 +240,16 @@ # we've already started reading the response return + if self._connection_reused: + # Use peek() to differentiate ECONNRESET with no response from + # ECONNRESET after a few bytes of response + try: + if not self.fp.peek(1): + msg = "Connection closed without response" + raise ConnectionClosed(msg) + except ConnectionError as err: + raise ConnectionClosed(err) from err + # read until we get a non-100 response while True: version, status, reason = self._read_status() @@ -694,6 +706,7 @@ self._tunnel_host = None self._tunnel_port = None self._tunnel_headers = {} + self._previous_response = False (self.host, self.port) = self._get_hostport(host, port) @@ -759,7 +772,8 @@ self.send(header_bytes) self.send(b'\r\n') - response = self.response_class(self.sock, method=self._method) + response = self.response_class(self.sock, method=self._method, + connection_reused=self._previous_response) (version, code, message) = response._read_status() if code != http.HTTPStatus.OK: @@ -783,6 +797,7 @@ self.sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) if self._tunnel_host: + self._previous_response = False self._tunnel() def close(self): @@ -804,6 +819,7 @@ if self.sock is None: if self.auto_open: self.connect() + self._previous_response = False else: raise NotConnected() @@ -1102,12 +1118,19 @@ if self.debuglevel > 0: response = self.response_class(self.sock, self.debuglevel, - method=self._method) + method=self._method, + connection_reused=self._previous_response) else: - response = self.response_class(self.sock, method=self._method) + response = self.response_class(self.sock, method=self._method, + connection_reused=self._previous_response) + self._previous_response = True try: - response.begin() + try: + response.begin() + except ConnectionClosed: + self.close() + raise assert response.will_close != _UNKNOWN self.__state = _CS_IDLE @@ -1239,5 +1262,10 @@ HTTPException.__init__(self, "got more than %d bytes when reading %s" % (_MAXLINE, line_type)) +class ConnectionClosed(BadStatusLine): + def __init__(self, *args): + BadStatusLine.__init__(self, "") + self.args = args + # for backwards compatibility error = HTTPException diff -r eaa38b75cc78 Lib/test/test_httplib.py --- a/Lib/test/test_httplib.py Fri Jan 23 17:30:26 2015 -0500 +++ b/Lib/test/test_httplib.py Sun Jan 25 00:37:09 2015 +0000 @@ -40,12 +40,19 @@ HOST = support.HOST +class RecvStream(io.RawIOBase): + def __init__(self, text): + self.reader = io.BytesIO(text) + def readable(self): + return True + def readinto(self, buffer): + return self.reader.readinto(buffer) + class FakeSocket: - def __init__(self, text, fileclass=io.BytesIO, host=None, port=None): + def __init__(self, text, fileclass=RecvStream, host=None, port=None): if isinstance(text, str): text = text.encode("ascii") - self.text = text - self.fileclass = fileclass + self.recv_stream = fileclass(text) self.data = b'' self.sendall_calls = 0 self.file_closed = False @@ -60,7 +67,7 @@ if mode != 'r' and mode != 'rb': 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 = io.BufferedReader(self.recv_stream) self.file.close = self.file_close #nerf close () return self.file @@ -88,23 +95,41 @@ def close(self): pass -class NoEOFBytesIO(io.BytesIO): - """Like BytesIO, but raises AssertionError on EOF. +class NoEOFRecvStream(RecvStream): + """Like RecvStream, but raises AssertionError on EOF. This is used below to test that http.client doesn't try to read more from the underlying file than it should. """ - def read(self, n=-1): - data = io.BytesIO.read(self, n) - if data == b'': + def readinto(self, buffer): + length = super().readinto(buffer) + if length == 0: raise AssertionError('caller tried to read past EOF') - return data + return length - def readline(self, length=None): - data = io.BytesIO.readline(self, length) - if data == b'': - raise AssertionError('caller tried to read past EOF') - return data +class TestHTTPConnection(client.HTTPConnection): + """HTTPConnection subclass using FakeSocket; counts connect() calls""" + + def __init__(self, *args): + self.connections = 0 + super().__init__('example.com') + self.fake_socket_args = args + self._create_connection = self.create_connection + + def connect(self): + """Count the number of times connect() is invoked""" + self.connections += 1 + return super().connect() + + def create_connection(self, *pos, **kw): + return FakeSocket(*self.fake_socket_args) + +BASIC_RESPONSE = ( + b'HTTP/1.1 200 OK\r\n' + b'Content-Length: 6\r\n' + b'\r\n' + b'body\r\n' +) class HeaderTests(TestCase): def test_auto_headers(self): @@ -364,7 +389,7 @@ 'HTTP/1.1 200 OK\r\n' 'Content-Length: 14432\r\n' '\r\n', - NoEOFBytesIO) + NoEOFRecvStream) resp = client.HTTPResponse(sock, method="HEAD") resp.begin() if resp.read(): @@ -377,7 +402,7 @@ 'HTTP/1.1 200 OK\r\n' 'Content-Length: 14432\r\n' '\r\n', - NoEOFBytesIO) + NoEOFRecvStream) resp = client.HTTPResponse(sock, method="HEAD") resp.begin() b = bytearray(5) @@ -400,7 +425,7 @@ with open(__file__, 'rb') as body: conn = client.HTTPConnection('example.com') - sock = FakeSocket(body) + sock = FakeSocket(b'') conn.sock = sock conn.request('GET', '/foo', body) self.assertTrue(sock.data.startswith(expected), '%r != %r' % @@ -607,13 +632,15 @@ self.fail('IncompleteRead expected') def test_epipe(self): - sock = EPipeSocket( - "HTTP/1.0 401 Authorization Required\r\n" - "Content-type: text/html\r\n" - "WWW-Authenticate: Basic realm=\"example\"\r\n", - b"Content-Length") conn = client.HTTPConnection("example.com") - conn.sock = sock + conn.sock = EPipeSocket(BASIC_RESPONSE, b"Content-Length") + conn.request("GET", "/open-persistent-connection") + conn.getresponse().read() + conn.sock.recv_stream = RecvStream( + b"HTTP/1.1 401 Authorization Required\r\n" + b"Content-type: text/html\r\n" + b"WWW-Authenticate: Basic realm=\"example\"\r\n" + ) self.assertRaises(OSError, lambda: conn.request("PUT", "/url", "body")) resp = conn.getresponse() @@ -992,6 +1019,105 @@ httpConn.close() +class PersistenceTest(TestCase): + def test_reuse_reconnect(self): + # Should reuse or reconnect depending on header from server + tests = ( + ('1.0', None, False), + ('1.0', 'keep-ALIVE', True), + ('1.1', None, True), + ('1.1', 'cloSE', False), + ) + for version, header, reuse in tests: + with self.subTest(version=version, header=header): + if header: + header = 'connecTION: {}\r\n'.format(header) + else: + header = '' + msg = ( + 'HTTP/{} 200 OK\r\n' + '{}' + 'Content-Length: 6\r\n' + '\r\n' + 'body\r\n' + ) + msg = msg.format(version, header) + conn = TestHTTPConnection(msg) + conn.request('GET', '/open-connection') + conn.getresponse().read() + # Check if socket is shut down and closed + self.assertEqual(conn.sock is not None, reuse) + self.assertEqual(conn.connections, 1) + conn.request('GET', '/subsequent-request') + if reuse: + self.assertEqual(conn.connections, 1) + else: + self.assertEqual(conn.connections, 2) + + def test_connection_closed(self): + class RecvOrResetStream(RecvStream): + def readinto(self, buffer): + length = super().readinto(buffer) + if not length: + raise ConnectionResetError() + return length + nonpersist_exception = (client.BadStatusLine, ConnectionError) + for stream_class in (RecvStream, RecvOrResetStream): + with self.subTest(stream_class): + # Should raise ConnectionClosed for >= second request + conn = TestHTTPConnection(BASIC_RESPONSE, stream_class) + conn.request('GET', '/open-persistent-connection') + conn.getresponse().read() + conn.request('GET', '/dropped-request') + self.assertRaises(client.ConnectionClosed, conn.getresponse) + self.assertEqual(conn.connections, 1) + + # HTTPConnection.connect() should be automatically invoked + def no_response_connection(*pos, **kw): + return FakeSocket(b'', stream_class) + conn._create_connection = no_response_connection + conn.request('GET', '/eof-after-reconnect') + self.assertEqual(conn.connections, 2) + + # No ConnectionClosed straight after reconnection + with self.assertRaises(nonpersist_exception) as caught: + conn.getresponse() + self.assertNotIsInstance(caught, client.ConnectionClosed) + + # No ConnectionClosed for initial request + conn = TestHTTPConnection(b'', stream_class) + conn.request('GET', '/eof-response') + with self.assertRaises(nonpersist_exception) as caught: + conn.getresponse() + self.assertNotIsInstance(caught, client.ConnectionClosed) + + # No ConnectionClosed if status line partly received + conn = TestHTTPConnection(BASIC_RESPONSE, stream_class) + conn.request('GET', '/open-persistent-connection') + conn.getresponse().read() + conn.sock.recv_stream = stream_class(b'H') + conn.request('GET', '/partial-status') + with self.assertRaises(nonpersist_exception) as caught: + conn.getresponse() + self.assertNotIsInstance(caught, client.ConnectionClosed) + + def test_100_close(self): + # No ConnectionClosed after 100 (Continue) response + conn = TestHTTPConnection(BASIC_RESPONSE) + conn.request('GET', '/open-persistent-connection') + conn.getresponse().read() + conn.sock.recv_stream = RecvStream( + b'HTTP/1.1 100 Continue\r\n' + b'\r\n' + # Missing final response + ) + conn.request('GET', '/100-continue-request', + headers={'Expect': '100-continue'}) + with self.assertRaises(client.BadStatusLine) as caught: + conn.getresponse() + self.assertNotIsInstance(caught, client.ConnectionClosed) + + class HTTPSTest(TestCase): def setUp(self): @@ -1298,6 +1424,7 @@ @support.reap_threads def test_main(verbose=None): support.run_unittest(HeaderTests, OfflineTest, BasicTest, TimeoutTest, + PersistenceTest, HTTPSTest, RequestBodyTest, SourceAddressTest, HTTPResponseTest, ExtendedReadTest, ExtendedReadTestChunked, TunnelTests) diff -r eaa38b75cc78 Lib/xmlrpc/client.py --- a/Lib/xmlrpc/client.py Fri Jan 23 17:30:26 2015 -0500 +++ b/Lib/xmlrpc/client.py Sun Jan 25 00:37:09 2015 +0000 @@ -1143,7 +1143,7 @@ if i or e.errno not in (errno.ECONNRESET, errno.ECONNABORTED, errno.EPIPE): raise - except http.client.BadStatusLine: #close after we sent request + except http.client.ConnectionClosed: #close without response if i: raise