gh-115997: Make read(N), read1, readinto and readline of HTTPResponse raise IncompleteRead

Lib/http/client.py

@@ -502,9 +502,9 @@ def read(self, amt=None):
                 amt = self.length
             s = self.fp.read(amt)
             if not s and amt:
-                # Ideally, we would raise IncompleteRead if the content-length
-                # wasn't satisfied, but it might break compatibility.
                 self._close_conn()
+                if self.length:
+                    raise IncompleteRead(s)
             elif self.length is not None:
                 self.length -= len(s)
                 if not self.length:
@@ -549,9 +549,9 @@ def readinto(self, b):
         # (for example, reading in 1k chunks)
         n = self.fp.readinto(b)
         if not n and b:
-            # Ideally, we would raise IncompleteRead if the content-length
-            # wasn't satisfied, but it might break compatibility.
             self._close_conn()
+            if self.length:
+                raise IncompleteRead(b"")
         elif self.length is not None:
             self.length -= n
             if not self.length:
@@ -715,6 +715,8 @@ def read1(self, n=-1):
         result = self.fp.read1(n)
         if not result and n:
             self._close_conn()
+            if self.length:
+                raise IncompleteRead(result)
         elif self.length is not None:
             self.length -= len(result)
             if not self.length:
@@ -741,10 +743,15 @@ def readline(self, limit=-1):
         result = self.fp.readline(limit)
         if not result and limit:
             self._close_conn()
+            if self.length:
+                raise IncompleteRead(result)
         elif self.length is not None:
             self.length -= len(result)
             if not self.length:
                 self._close_conn()
+            elif len(result) != limit and not result.endswith(b"\n"):
+                self._close_conn()
+                raise IncompleteRead(result)
         return result
 
     def _read1_chunked(self, n):

Lib/test/test_httplib.py

@@ -8,6 +8,7 @@
 import re
 import socket
 import threading
+from functools import partial
 
 import unittest
 from unittest import mock
@@ -1012,7 +1013,11 @@ def test_partial_reads_incomplete_body(self):
         self.assertEqual(resp.read(2), b'Te')
         self.assertFalse(resp.isclosed())
         self.assertEqual(resp.read(2), b'xt')
-        self.assertEqual(resp.read(1), b'')
+        with self.assertRaises(client.IncompleteRead) as cm:
+            resp.read(1)
+        exception = cm.exception
+        self.assertEqual(exception.partial, b"")
+        self.assertIsNone(exception.expected)
         self.assertTrue(resp.isclosed())
 
     def test_partial_readintos_incomplete_body(self):
@@ -1030,8 +1035,11 @@ def test_partial_readintos_incomplete_body(self):
         n = resp.readinto(b)
         self.assertEqual(n, 2)
         self.assertEqual(bytes(b), b'xt')
-        n = resp.readinto(b)
-        self.assertEqual(n, 0)
+        with self.assertRaises(client.IncompleteRead) as cm:
+            resp.readinto(b)
+        exception = cm.exception
+        self.assertEqual(exception.partial, b"")
+        self.assertIsNone(exception.expected)
         self.assertTrue(resp.isclosed())
         self.assertFalse(resp.closed)
         resp.close()
@@ -1884,6 +1892,78 @@ class ExtendedReadTestContentLengthKnown(ExtendedReadTest):
     _header, _body = ExtendedReadTest.lines.split('\r\n\r\n', 1)
     lines = _header + f'\r\nContent-Length: {len(_body)}\r\n\r\n' + _body
 
+    def _test_incomplete_read(self, read_meth, expected_none):
+        resp = self.resp
+        # Reduce the size of content the response object will read to
+        # cause the incomplete read.
+        resp.fp.read(1)
+        with self.assertRaises(client.IncompleteRead) as cm:
+            while True:
+                data = read_meth()
+                if not data:
+                    break
+        exception = cm.exception
+        # Unlike `read1` and `readline`, `read` tries to read the whole
+        # content during one call, so it's partial is not empty in this
+        # case.
+        # `read1` and `readline` raise `IncompleteRead` only when they
+        # read 0 bytes before all expected content has been read, so the
+        # partial is empty.
+        if read_meth == self.resp.read:
+            expected_partial = self._body[1:].encode()
+        else:
+            expected_partial = b""
+        self.assertEqual(exception.partial, expected_partial)
+        if expected_none:
+            self.assertIsNone(exception.expected)
+        else:
+            self.assertEqual(exception.expected, 1)
+        self.assertTrue(resp.isclosed())
+
+    def test_read_incomplete_read(self):
+        self._test_incomplete_read(self.resp.read, expected_none=False)
+
+    def test_read_incomplete_read_n(self):
+        read_meth = partial(self.resp.read, len(self._body))
+        self._test_incomplete_read(read_meth, expected_none=True)
+
+    def test_read1_incomplete_read(self):
+        self._test_incomplete_read(self.resp.read1, expected_none=True)
+
+    def test_readline_incomplete_read_with_complete_line(self):
+        """
+        Test that IncompleteRead is raised when readline finishes
+        reading a response but the needed content length is not reached.
+        """
+        resp = self.resp
+        content = resp.fp.read()
+        # For this test case, we must ensure that the last byte read
+        # will be a newline. There is a different handling of readline
+        # not reaching a newline.
+        content = content[:-1] + b"\n"
+        resp.fp = io.BytesIO(content)
+        self._test_incomplete_read(self.resp.readline, expected_none=True)
+
+    def test_readline_incomplete_read_with_incomplete_line(self):
+        """
+        Test that IncompleteRead is raised when readline is expected
+        to read a line fully but a newline is not reached.
+        """
+        resp = self.resp
+        content = resp.fp.read()
+        # Remove last newline and following bytes.
+        content = content.rsplit(b'\n', 1)[0]
+        resp.fp = io.BytesIO(content)
+        with self.assertRaises(client.IncompleteRead) as cm:
+            while True:
+                data = resp.readline()
+                if not data:
+                    break
+        exception = cm.exception
+        self.assertEqual(exception.partial, content.rsplit(b'\n', 1)[1])
+        self.assertIsNone(exception.expected)
+        self.assertTrue(resp.isclosed())
+
 
 class ExtendedReadTestChunked(ExtendedReadTest):
     """

Lib/urllib/request.py

@@ -237,7 +237,15 @@ def urlretrieve(url, filename=None, reporthook=None, data=None):
             if reporthook:
                 reporthook(blocknum, bs, size)
 
-            while block := fp.read(bs):
+            while True:
+                try:
+                    block = fp.read(bs)
+                except http.client.IncompleteRead:
+                    # This case has been handled by ContentTooShortError
+                    # below historically.
+                    break
+                if not block:
+                    break
                 read += len(block)
                 tfp.write(block)
                 blocknum += 1

Misc/NEWS.d/next/Library/2024-02-27-20-56-44.gh-issue-115997.LqLP4V.rst

@@ -0,0 +1,6 @@
+Make ``read(N)``, ``read1``, ``readinto`` and ``readline`` of
+``http.client.HTTPResponse`` raise :exc:`http.client.IncompleteRead`
+instead of returning zero bytes if a connection is closed before an expected
+number of bytes has been read. Also, ``http.client.HTTPResponse.readline``
+raises when an expected newline has not been reached.
+Patch by Illia Volochii and Seth Larson.