#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.)

Doc/library/http.client.rst

@@ -175,6 +175,17 @@ The following exceptions are raised as appropriate:
    is received in the HTTP protocol from the server.
 
 
+.. exception:: RemoteDisconnected
+
+   A subclass of :exc:`ConnectionResetError` and :exc:`BadStatusLine`.  Raised
+   by :meth:`HTTPConnection.getresponse` when the attempt to read the response
+   results in no data read from the connection, indicating that the remote end
+   has closed the connection.
+
+   .. versionadded:: 3.5
+      Previously, :exc:`BadStatusLine`\ ``('')`` was raised.
+
+
 The constants defined in this module are:
 
 .. data:: HTTP_PORT
@@ -247,6 +258,11 @@ HTTPConnection Objects
       Note that you must have read the whole response before you can send a new
       request to the server.
 
+   .. versionchanged:: 3.5
+      If a :exc:`ConnectionError` or subclass is raised, the
+      :class:`HTTPConnection` object will be ready to reconnect when
+      a new request is sent.
+
 
 .. method:: HTTPConnection.set_debuglevel(level)
 
@@ -285,7 +301,9 @@ HTTPConnection Objects
 
 .. method:: HTTPConnection.connect()
 
-   Connect to the server specified when the object was created.
+   Connect to the server specified when the object was created.  By default,
+   this is called automatically when making a request if the client does not
+   already have a connection.
 
 
 .. method:: HTTPConnection.close()

Lib/http/client.py

@@ -20,10 +20,12 @@ request. This diagram details these state transitions:
       | ( 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 @@ __all__ = ["HTTPResponse", "HTTPConnection",
            "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 @@ class HTTPResponse(io.BufferedIOBase):
         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:
@@ -1159,8 +1163,12 @@ class HTTPConnection:
         else:
             response = self.response_class(self.sock, method=self._method)
 
+        try:
             try:
                 response.begin()
+            except ConnectionError:
+                self.close()
+                raise
             assert response.will_close != _UNKNOWN
             self.__state = _CS_IDLE
 
@@ -1292,5 +1300,10 @@ class LineTooLong(HTTPException):
         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

Lib/test/test_httplib.py

@@ -107,6 +107,23 @@ class NoEOFBytesIO(io.BytesIO):
             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 @@ class BasicTest(TestCase):
                 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 @@ class TimeoutTest(TestCase):
         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 @@ class TunnelTests(TestCase):
 @support.reap_threads
 def test_main(verbose=None):
     support.run_unittest(HeaderTests, OfflineTest, BasicTest, TimeoutTest,
+                         PersistenceTest,
                          HTTPSTest, RequestBodyTest, SourceAddressTest,
                          HTTPResponseTest, ExtendedReadTest,
                          ExtendedReadTestChunked, TunnelTests)

Lib/xmlrpc/client.py

@@ -1128,7 +1128,7 @@ class Transport:
                 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