Issue #16298: In HTTPResponse.read(), close the socket when there is no...

Issue #16298: In HTTPResponse.read(), close the socket when there is no Content-Length and the incoming stream is finished.
Patch by Eran Rundstein.

Lib/http/client.py

@@ -536,6 +536,9 @@ class HTTPResponse(io.RawIOBase):
             self.length -= n
             if not self.length:
                 self.close()
+        else:
+            if not n:
+                self.close()
         return n
 
     def _read_next_chunk_size(self):

Lib/test/test_httplib.py

@@ -175,7 +175,7 @@ class BasicTest(TestCase):
         self.assertEqual(repr(exc), '''BadStatusLine("\'\'",)''')
 
     def test_partial_reads(self):
-        # if we have a lenght, the system knows when to close itself
+        # if we have a length, the system knows when to close itself
         # same behaviour than when we read the whole thing with read()
         body = "HTTP/1.1 200 Ok\r\nContent-Length: 4\r\n\r\nText"
         sock = FakeSocket(body)
@@ -187,7 +187,7 @@ class BasicTest(TestCase):
         self.assertTrue(resp.isclosed())
 
     def test_partial_readintos(self):
-        # if we have a lenght, the system knows when to close itself
+        # if we have a length, the system knows when to close itself
         # same behaviour than when we read the whole thing with read()
         body = "HTTP/1.1 200 Ok\r\nContent-Length: 4\r\n\r\nText"
         sock = FakeSocket(body)
@@ -203,6 +203,38 @@ class BasicTest(TestCase):
         self.assertEqual(bytes(b), b'xt')
         self.assertTrue(resp.isclosed())
 
+    def test_partial_reads_no_content_length(self):
+        # when no length is present, the socket should be gracefully closed when
+        # all data was read
+        body = "HTTP/1.1 200 Ok\r\n\r\nText"
+        sock = FakeSocket(body)
+        resp = client.HTTPResponse(sock)
+        resp.begin()
+        self.assertEqual(resp.read(2), b'Te')
+        self.assertFalse(resp.isclosed())
+        self.assertEqual(resp.read(2), b'xt')
+        self.assertEqual(resp.read(1), b'')
+        self.assertTrue(resp.isclosed())
+
+    def test_partial_readintos_no_content_length(self):
+        # when no length is present, the socket should be gracefully closed when
+        # all data was read
+        body = "HTTP/1.1 200 Ok\r\n\r\nText"
+        sock = FakeSocket(body)
+        resp = client.HTTPResponse(sock)
+        resp.begin()
+        b = bytearray(2)
+        n = resp.readinto(b)
+        self.assertEqual(n, 2)
+        self.assertEqual(bytes(b), b'Te')
+        self.assertFalse(resp.isclosed())
+        n = resp.readinto(b)
+        self.assertEqual(n, 2)
+        self.assertEqual(bytes(b), b'xt')
+        n = resp.readinto(b)
+        self.assertEqual(n, 0)
+        self.assertTrue(resp.isclosed())
+
     def test_host_port(self):
         # Check invalid host_port
 

Misc/ACKS

@@ -1016,6 +1016,7 @@ Paul Rubin
 Sam Ruby
 Demur Rumed
 Audun S. Runde
+Eran Rundstein
 Rauli Ruohonen
 Jeff Rush
 Sam Rushing

Misc/NEWS

@@ -108,6 +108,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #16298: In HTTPResponse.read(), close the socket when there is no
+  Content-Length and the incoming stream is finished.  Patch by Eran
+  Rundstein.
+
 - Issue #15872: Fix 3.3 regression introduced by the new fd-based shutil.rmtree
   that caused it to not ignore certain errors when ignore_errors was set.
   Patch by Alessandro Moura and Serhiy Storchaka.