From 9878730e20d68eb2a04ed82c5fffff9ab8f94d02 Mon Sep 17 00:00:00 2001 From: Illia Volochii Date: Tue, 27 Feb 2024 20:48:50 +0200 Subject: [PATCH 1/5] gh-115997: Make `HTTPResponse.read1` and `readline` raise `IncompleteRead` --- Lib/http/client.py | 4 ++++ Lib/test/test_httplib.py | 24 +++++++++++++++++++ ...-02-27-20-56-44.gh-issue-115997.LqLP4V.rst | 4 ++++ 3 files changed, 32 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2024-02-27-20-56-44.gh-issue-115997.LqLP4V.rst diff --git a/Lib/http/client.py b/Lib/http/client.py index a353716a8506e6..4a5f97e5ff8bcc 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -663,6 +663,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, self.length) elif self.length is not None: self.length -= len(result) if not self.length: @@ -689,6 +691,8 @@ def readline(self, limit=-1): result = self.fp.readline(limit) if not result and limit: self._close_conn() + if self.length: + raise IncompleteRead(result, self.length) elif self.length is not None: self.length -= len(result) if not self.length: diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index 6e63a8872d9c6e..eb776833df4d2a 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -1608,6 +1608,30 @@ 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): + 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 = resp.read1() + if not data: + break + exception = cm.exception + self.assertEqual(exception.partial, b"") + self.assertEqual(exception.expected, 1) + self.assertTrue(resp.isclosed()) + + def test_read_incomplete_read(self): + self._test_incomplete_read(self.resp.read) + + def test_read1_incomplete_read(self): + self._test_incomplete_read(self.resp.read1) + + def test_readline_incomplete_read(self): + self._test_incomplete_read(self.resp.readline) + class ExtendedReadTestChunked(ExtendedReadTest): """ diff --git a/Misc/NEWS.d/next/Library/2024-02-27-20-56-44.gh-issue-115997.LqLP4V.rst b/Misc/NEWS.d/next/Library/2024-02-27-20-56-44.gh-issue-115997.LqLP4V.rst new file mode 100644 index 00000000000000..f33feb0390d1cc --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-02-27-20-56-44.gh-issue-115997.LqLP4V.rst @@ -0,0 +1,4 @@ +Make ``http.client.HTTPResponse.read1`` and +``http.client.HTTPResponse.readline`` raise :exc:`IncompleteRead` instead of +returning zero bytes if a connection is closed before an expected number of +bytes has been read. Patch by Illia Volochii. From 9f8f278604a5a0ec068d604d48c93a97e993bb4c Mon Sep 17 00:00:00 2001 From: Illia Volochii Date: Tue, 27 Feb 2024 21:05:57 +0200 Subject: [PATCH 2/5] Fix the new tests --- Lib/test/test_httplib.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index eb776833df4d2a..c04159f4bfb667 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -1615,11 +1615,21 @@ def _test_incomplete_read(self, read_meth): resp.fp.read(1) with self.assertRaises(client.IncompleteRead) as cm: while True: - data = resp.read1() + data = read_meth() if not data: break exception = cm.exception - self.assertEqual(exception.partial, b"") + # 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) self.assertEqual(exception.expected, 1) self.assertTrue(resp.isclosed()) From 33f7d775821a5de7a4c45636d38ba7b1463baf3f Mon Sep 17 00:00:00 2001 From: Illia Volochii Date: Mon, 10 Feb 2025 20:54:03 +0200 Subject: [PATCH 3/5] Do not set `expected` to new `IncompleteRead` --- Lib/http/client.py | 4 ++-- Lib/test/test_httplib.py | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/Lib/http/client.py b/Lib/http/client.py index 81ca7fca53b5de..819654d62fd12d 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -666,7 +666,7 @@ def read1(self, n=-1): if not result and n: self._close_conn() if self.length: - raise IncompleteRead(result, self.length) + raise IncompleteRead(result) elif self.length is not None: self.length -= len(result) if not self.length: @@ -694,7 +694,7 @@ def readline(self, limit=-1): if not result and limit: self._close_conn() if self.length: - raise IncompleteRead(result, self.length) + raise IncompleteRead(result) elif self.length is not None: self.length -= len(result) if not self.length: diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index 13a1acd678ac65..4d847bb368f382 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -1641,7 +1641,7 @@ 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): + 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. @@ -1663,17 +1663,20 @@ def _test_incomplete_read(self, read_meth): else: expected_partial = b"" self.assertEqual(exception.partial, expected_partial) - self.assertEqual(exception.expected, 1) + 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) + self._test_incomplete_read(self.resp.read, expected_none=False) def test_read1_incomplete_read(self): - self._test_incomplete_read(self.resp.read1) + self._test_incomplete_read(self.resp.read1, expected_none=True) def test_readline_incomplete_read(self): - self._test_incomplete_read(self.resp.readline) + self._test_incomplete_read(self.resp.readline, expected_none=True) class ExtendedReadTestChunked(ExtendedReadTest): From 26b43e16a0580bbeb6902cb15e329138f53756b8 Mon Sep 17 00:00:00 2001 From: Illia Volochii Date: Mon, 10 Feb 2025 23:03:57 +0200 Subject: [PATCH 4/5] Handle a case with a not reached newline --- Lib/http/client.py | 3 ++ Lib/test/test_httplib.py | 33 ++++++++++++++++++- ...-02-27-20-56-44.gh-issue-115997.LqLP4V.rst | 7 ++-- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/Lib/http/client.py b/Lib/http/client.py index 819654d62fd12d..5abaabdd3de096 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -699,6 +699,9 @@ def readline(self, limit=-1): 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): diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index 4d847bb368f382..2cf0cd9b8c8e2b 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -1675,9 +1675,40 @@ def test_read_incomplete_read(self): def test_read1_incomplete_read(self): self._test_incomplete_read(self.resp.read1, expected_none=True) - def test_readline_incomplete_read(self): + 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() + # Truncate the content to the last newline. + content = content[:content.rindex(b"\n") - 1] + 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.split(b"\n")[-1]) + self.assertIsNone(exception.expected) + self.assertTrue(resp.isclosed()) + class ExtendedReadTestChunked(ExtendedReadTest): """ diff --git a/Misc/NEWS.d/next/Library/2024-02-27-20-56-44.gh-issue-115997.LqLP4V.rst b/Misc/NEWS.d/next/Library/2024-02-27-20-56-44.gh-issue-115997.LqLP4V.rst index f33feb0390d1cc..9fd6e790ce4af9 100644 --- a/Misc/NEWS.d/next/Library/2024-02-27-20-56-44.gh-issue-115997.LqLP4V.rst +++ b/Misc/NEWS.d/next/Library/2024-02-27-20-56-44.gh-issue-115997.LqLP4V.rst @@ -1,4 +1,5 @@ Make ``http.client.HTTPResponse.read1`` and -``http.client.HTTPResponse.readline`` raise :exc:`IncompleteRead` instead of -returning zero bytes if a connection is closed before an expected number of -bytes has been read. Patch by Illia Volochii. +``http.client.HTTPResponse.readline`` 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. From 0b6c92983e6bd74611c9c8959d993874bc50d9f1 Mon Sep 17 00:00:00 2001 From: Illia Volochii Date: Tue, 1 Apr 2025 20:47:44 +0300 Subject: [PATCH 5/5] Refactor `test_readline_incomplete_read_with_incomplete_line` --- Lib/test/test_httplib.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index 2cf0cd9b8c8e2b..3ec1b235e7dc69 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -1696,8 +1696,8 @@ def test_readline_incomplete_read_with_incomplete_line(self): """ resp = self.resp content = resp.fp.read() - # Truncate the content to the last newline. - content = content[:content.rindex(b"\n") - 1] + # 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: @@ -1705,7 +1705,7 @@ def test_readline_incomplete_read_with_incomplete_line(self): if not data: break exception = cm.exception - self.assertEqual(exception.partial, content.split(b"\n")[-1]) + self.assertEqual(exception.partial, content.rsplit(b'\n', 1)[1]) self.assertIsNone(exception.expected) self.assertTrue(resp.isclosed())