#3566: Clean up handling of remote server disconnects.
This changeset does two things: introduces a new RemoteDisconnected exception
(that subclasses ConnectionResetError and BadStatusLine) so that a remote
server disconnection can be detected by client code (and provides a better
error message for debugging purposes), and ensures that the client socket is
closed if a ConnectionError happens, so that the automatic re-connection code
can work if the application handles the error and continues on.
Tests are added that confirm that a connection is re-used or not re-used
as appropriate to the various combinations of protocol version and headers.
Patch by Martin Panter, reviewed by Demian Brecht. (Tweaked only slightly by
me.)
diff --git a/Lib/http/client.py b/Lib/http/client.py
index f5889ed..ac120e7 100644
--- a/Lib/http/client.py
+++ b/Lib/http/client.py
@@ -20,10 +20,12 @@
| ( putheader() )* endheaders()
v
Request-sent
- |
- | response = getresponse()
- v
- Unread-response [Response-headers-read]
+ |\_____________________________
+ | | getresponse() raises
+ | response = getresponse() | ConnectionError
+ v v
+ Unread-response Idle
+ [Response-headers-read]
|\____________________
| |
| response.read() | putrequest()
@@ -83,7 +85,8 @@
"UnknownTransferEncoding", "UnimplementedFileMode",
"IncompleteRead", "InvalidURL", "ImproperConnectionState",
"CannotSendRequest", "CannotSendHeader", "ResponseNotReady",
- "BadStatusLine", "LineTooLong", "error", "responses"]
+ "BadStatusLine", "LineTooLong", "RemoteDisconnected", "error",
+ "responses"]
HTTP_PORT = 80
HTTPS_PORT = 443
@@ -245,7 +248,8 @@
if not line:
# Presumably, the server closed the connection before
# sending a valid response.
- raise BadStatusLine(line)
+ raise RemoteDisconnected("Remote end closed connection without"
+ " response")
try:
version, status, reason = line.split(None, 2)
except ValueError:
@@ -1160,7 +1164,11 @@
response = self.response_class(self.sock, method=self._method)
try:
- response.begin()
+ try:
+ response.begin()
+ except ConnectionError:
+ self.close()
+ raise
assert response.will_close != _UNKNOWN
self.__state = _CS_IDLE
@@ -1292,5 +1300,10 @@
HTTPException.__init__(self, "got more than %d bytes when reading %s"
% (_MAXLINE, line_type))
+class RemoteDisconnected(ConnectionResetError, BadStatusLine):
+ def __init__(self, *pos, **kw):
+ BadStatusLine.__init__(self, "")
+ ConnectionResetError.__init__(self, *pos, **kw)
+
# for backwards compatibility
error = HTTPException
diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
index b4ec63c..d809414 100644
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@ -107,6 +107,23 @@
raise AssertionError('caller tried to read past EOF')
return data
+class FakeSocketHTTPConnection(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)
+
class HeaderTests(TestCase):
def test_auto_headers(self):
# Some headers are added automatically, but should not be added by
@@ -777,7 +794,7 @@
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.sock = FakeSocket('Invalid status line')
conn.request('GET', '/')
self.assertRaises(client.BadStatusLine, conn.getresponse)
self.assertTrue(response.closed)
@@ -1174,6 +1191,78 @@
httpConn.close()
+class PersistenceTest(TestCase):
+
+ def test_reuse_reconnect(self):
+ # Should reuse or reconnect depending on header from server
+ tests = (
+ ('1.0', '', False),
+ ('1.0', 'Connection: keep-alive\r\n', True),
+ ('1.1', '', True),
+ ('1.1', 'Connection: close\r\n', False),
+ ('1.0', 'Connection: keep-ALIVE\r\n', True),
+ ('1.1', 'Connection: cloSE\r\n', False),
+ )
+ for version, header, reuse in tests:
+ with self.subTest(version=version, header=header):
+ msg = (
+ 'HTTP/{} 200 OK\r\n'
+ '{}'
+ 'Content-Length: 12\r\n'
+ '\r\n'
+ 'Dummy body\r\n'
+ ).format(version, header)
+ conn = FakeSocketHTTPConnection(msg)
+ self.assertIsNone(conn.sock)
+ conn.request('GET', '/open-connection')
+ with conn.getresponse() as response:
+ self.assertEqual(conn.sock is None, not reuse)
+ response.read()
+ self.assertEqual(conn.sock is None, not reuse)
+ self.assertEqual(conn.connections, 1)
+ conn.request('GET', '/subsequent-request')
+ self.assertEqual(conn.connections, 1 if reuse else 2)
+
+ def test_disconnected(self):
+
+ def make_reset_reader(text):
+ """Return BufferedReader that raises ECONNRESET at EOF"""
+ stream = io.BytesIO(text)
+ def readinto(buffer):
+ size = io.BytesIO.readinto(stream, buffer)
+ if size == 0:
+ raise ConnectionResetError()
+ return size
+ stream.readinto = readinto
+ return io.BufferedReader(stream)
+
+ tests = (
+ (io.BytesIO, client.RemoteDisconnected),
+ (make_reset_reader, ConnectionResetError),
+ )
+ for stream_factory, exception in tests:
+ with self.subTest(exception=exception):
+ conn = FakeSocketHTTPConnection(b'', stream_factory)
+ conn.request('GET', '/eof-response')
+ self.assertRaises(exception, conn.getresponse)
+ self.assertIsNone(conn.sock)
+ # HTTPConnection.connect() should be automatically invoked
+ conn.request('GET', '/reconnect')
+ self.assertEqual(conn.connections, 2)
+
+ def test_100_close(self):
+ conn = FakeSocketHTTPConnection(
+ b'HTTP/1.1 100 Continue\r\n'
+ b'\r\n'
+ # Missing final response
+ )
+ conn.request('GET', '/', headers={'Expect': '100-continue'})
+ self.assertRaises(client.RemoteDisconnected, conn.getresponse)
+ self.assertIsNone(conn.sock)
+ conn.request('GET', '/reconnect')
+ self.assertEqual(conn.connections, 2)
+
+
class HTTPSTest(TestCase):
def setUp(self):
@@ -1513,6 +1602,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 --git a/Lib/xmlrpc/client.py b/Lib/xmlrpc/client.py
index 34208d1..da089a2 100644
--- a/Lib/xmlrpc/client.py
+++ b/Lib/xmlrpc/client.py
@@ -1128,7 +1128,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.RemoteDisconnected:
if i:
raise