Issue #16723: httplib.HTTPResponse no longer marked closed when the connection

is automatically closed.

Lib/http/client.py

@@ -332,7 +332,7 @@ class HTTPResponse(io.RawIOBase):
                 # empty version will cause next test to fail.
                 version = ""
         if not version.startswith("HTTP/"):
-            self.close()
+            self._close_conn()
             raise BadStatusLine(line)
 
         # The status code is a three-digit number
@@ -454,22 +454,25 @@ class HTTPResponse(io.RawIOBase):
         # otherwise, assume it will close
         return True
 
+    def _close_conn(self):
+        fp = self.fp
+        self.fp = None
+        fp.close()
+
     def close(self):
+        super().close() # set "closed" flag
         if self.fp:
-            self.fp.close()
+            self._close_conn()
-            self.fp = None
 
     # These implementations are for the benefit of io.BufferedReader.
 
     # XXX This class should probably be revised to act more like
     # the "raw stream" that BufferedReader expects.
 
-    @property
-    def closed(self):
-        return self.isclosed()
-
     def flush(self):
-        self.fp.flush()
+        super().flush()
+        if self.fp:
+            self.fp.flush()
 
     def readable(self):
         return True
@@ -477,6 +480,7 @@ class HTTPResponse(io.RawIOBase):
     # End of "raw stream" methods
 
     def isclosed(self):
+        """True if the connection is closed."""
         # NOTE: it is possible that we will not ever call self.close(). This
         #       case occurs when will_close is TRUE, length is None, and we
         #       read up to the last byte, but NOT past it.
@@ -490,7 +494,7 @@ class HTTPResponse(io.RawIOBase):
             return b""
 
         if self._method == "HEAD":
-            self.close()
+            self._close_conn()
             return b""
 
         if amt is not None:
@@ -510,10 +514,10 @@ class HTTPResponse(io.RawIOBase):
                 try:
                     s = self._safe_read(self.length)
                 except IncompleteRead:
-                    self.close()
+                    self._close_conn()
                     raise
                 self.length = 0
-            self.close()        # we read everything
+            self._close_conn()        # we read everything
             return s
 
     def readinto(self, b):
@@ -521,7 +525,7 @@ class HTTPResponse(io.RawIOBase):
             return 0
 
         if self._method == "HEAD":
-            self.close()
+            self._close_conn()
             return 0
 
         if self.chunked:
@@ -539,11 +543,11 @@ class HTTPResponse(io.RawIOBase):
         if not n:
             # Ideally, we would raise IncompleteRead if the content-length
             # wasn't satisfied, but it might break compatibility.
-            self.close()
+            self._close_conn()
         elif self.length is not None:
             self.length -= n
             if not self.length:
-                self.close()
+                self._close_conn()
         return n
 
     def _read_next_chunk_size(self):
@@ -559,7 +563,7 @@ class HTTPResponse(io.RawIOBase):
         except ValueError:
             # close the connection as protocol synchronisation is
             # probably lost
-            self.close()
+            self._close_conn()
             raise
 
     def _read_and_discard_trailer(self):
@@ -597,7 +601,7 @@ class HTTPResponse(io.RawIOBase):
         self._read_and_discard_trailer()
 
         # we read everything; close the "file"
-        self.close()
+        self._close_conn()
 
         return b''.join(value)
 
@@ -638,7 +642,7 @@ class HTTPResponse(io.RawIOBase):
         self._read_and_discard_trailer()
 
         # we read everything; close the "file"
-        self.close()
+        self._close_conn()
 
         return total_bytes
 

Lib/test/test_httplib.py

@@ -166,6 +166,9 @@ class BasicTest(TestCase):
         resp.begin()
         self.assertEqual(resp.read(), b"Text")
         self.assertTrue(resp.isclosed())
+        self.assertFalse(resp.closed)
+        resp.close()
+        self.assertTrue(resp.closed)
 
         body = "HTTP/1.1 400.100 Not Ok\r\n\r\nText"
         sock = FakeSocket(body)
@@ -187,6 +190,9 @@ class BasicTest(TestCase):
         self.assertFalse(resp.isclosed())
         self.assertEqual(resp.read(2), b'xt')
         self.assertTrue(resp.isclosed())
+        self.assertFalse(resp.closed)
+        resp.close()
+        self.assertTrue(resp.closed)
 
     def test_partial_readintos(self):
         # if we have a length, the system knows when to close itself
@@ -204,6 +210,9 @@ class BasicTest(TestCase):
         self.assertEqual(n, 2)
         self.assertEqual(bytes(b), b'xt')
         self.assertTrue(resp.isclosed())
+        self.assertFalse(resp.closed)
+        resp.close()
+        self.assertTrue(resp.closed)
 
     def test_partial_reads_no_content_length(self):
         # when no length is present, the socket should be gracefully closed when
@@ -217,6 +226,9 @@ class BasicTest(TestCase):
         self.assertEqual(resp.read(2), b'xt')
         self.assertEqual(resp.read(1), b'')
         self.assertTrue(resp.isclosed())
+        self.assertFalse(resp.closed)
+        resp.close()
+        self.assertTrue(resp.closed)
 
     def test_partial_readintos_no_content_length(self):
         # when no length is present, the socket should be gracefully closed when
@@ -268,6 +280,9 @@ class BasicTest(TestCase):
         n = resp.readinto(b)
         self.assertEqual(n, 0)
         self.assertTrue(resp.isclosed())
+        self.assertFalse(resp.closed)
+        resp.close()
+        self.assertTrue(resp.closed)
 
     def test_host_port(self):
         # Check invalid host_port
@@ -495,6 +510,9 @@ class BasicTest(TestCase):
         self.assertEqual(resp.status, 200)
         self.assertEqual(resp.reason, 'OK')
         self.assertTrue(resp.isclosed())
+        self.assertFalse(resp.closed)
+        resp.close()
+        self.assertTrue(resp.closed)
 
     def test_readinto_chunked_head(self):
         chunked_start = (
@@ -515,6 +533,9 @@ class BasicTest(TestCase):
         self.assertEqual(resp.status, 200)
         self.assertEqual(resp.reason, 'OK')
         self.assertTrue(resp.isclosed())
+        self.assertFalse(resp.closed)
+        resp.close()
+        self.assertTrue(resp.closed)
 
     def test_negative_content_length(self):
         sock = FakeSocket(
@@ -590,6 +611,9 @@ class BasicTest(TestCase):
         resp.begin()
         self.assertEqual(resp.read(), b'')
         self.assertTrue(resp.isclosed())
+        self.assertFalse(resp.closed)
+        resp.close()
+        self.assertTrue(resp.closed)
 
     def test_delayed_ack_opt(self):
         # Test that Nagle/delayed_ack optimistaion works correctly.

Misc/NEWS

@@ -235,6 +235,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #16723: httplib.HTTPResponse no longer marked closed when the connection
+  is automatically closed.
+
 - Issue #15359: Add CAN_BCM protocol support to the socket module. Patch by
   Brian Thorne.