From ce911c3fed7e6404fba7de3868b1000425984b1e Mon Sep 17 00:00:00 2001 From: Martin Panter Date: Thu, 17 Mar 2016 06:42:48 +0000 Subject: Issue #26499: Fixes to HTTPResponse.readline() and read1(), by Silent Ghost --- Doc/library/http.client.rst | 4 +++ Lib/http/client.py | 8 +++++ Lib/test/test_httplib.py | 79 +++++++++++++++++++++++++++++++++++++++------ Misc/NEWS | 4 +++ 4 files changed, 86 insertions(+), 9 deletions(-) diff --git a/Doc/library/http.client.rst b/Doc/library/http.client.rst index f42afe9..649abd1 100644 --- a/Doc/library/http.client.rst +++ b/Doc/library/http.client.rst @@ -362,6 +362,10 @@ server. It provides access to the request headers and the entity body. The response is an iterable object and can be used in a with statement. +.. versionchanged:: 3.5 + The :class:`io.BufferedIOBase` interface is now implemented and + all of its reader operations are supported. + .. method:: HTTPResponse.read([amt]) diff --git a/Lib/http/client.py b/Lib/http/client.py index 24231b5..2d6e07b 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -635,6 +635,8 @@ class HTTPResponse(io.BufferedIOBase): return b"" if self.chunked: return self._read1_chunked(n) + if self.length is not None and (n < 0 or n > self.length): + n = self.length try: result = self.fp.read1(n) except ValueError: @@ -645,6 +647,8 @@ class HTTPResponse(io.BufferedIOBase): result = self.fp.read1(16*1024) if not result and n: self._close_conn() + elif self.length is not None: + self.length -= len(result) return result def peek(self, n=-1): @@ -662,9 +666,13 @@ class HTTPResponse(io.BufferedIOBase): if self.chunked: # Fallback to IOBase readline which uses peek() and read() return super().readline(limit) + if self.length is not None and (limit < 0 or limit > self.length): + limit = self.length result = self.fp.readline(limit) if not result and limit: self._close_conn() + elif self.length is not None: + self.length -= len(result) return result def _read1_chunked(self, n): diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index 295b9fb..6d2ed39 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -341,8 +341,8 @@ class BasicTest(TestCase): self.assertEqual(repr(exc), '''BadStatusLine("\'\'",)''') def test_partial_reads(self): - # if we have a length, the system knows when to close itself - # same behaviour than when we read the whole thing with read() + # if we have Content-Length, HTTPResponse knows when to close itself, + # the same behaviour as 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) resp = client.HTTPResponse(sock) @@ -355,9 +355,24 @@ class BasicTest(TestCase): resp.close() self.assertTrue(resp.closed) + def test_mixed_reads(self): + # readline() should update the remaining length, so that read() knows + # how much data is left and does not raise IncompleteRead + body = "HTTP/1.1 200 Ok\r\nContent-Length: 13\r\n\r\nText\r\nAnother" + sock = FakeSocket(body) + resp = client.HTTPResponse(sock) + resp.begin() + self.assertEqual(resp.readline(), b'Text\r\n') + self.assertFalse(resp.isclosed()) + self.assertEqual(resp.read(), b'Another') + 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 - # same behaviour than when we read the whole thing with read() + # if we have Content-Length, HTTPResponse knows when to close itself, + # the same behaviour as 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) resp = client.HTTPResponse(sock) @@ -827,7 +842,7 @@ class BasicTest(TestCase): resp.begin() self.assertEqual(resp.read(), expected) # we should have reached the end of the file - self.assertEqual(sock.file.read(100), b"") #we read to the end + self.assertEqual(sock.file.read(), b"") #we read to the end resp.close() def test_chunked_sync(self): @@ -839,19 +854,65 @@ class BasicTest(TestCase): resp.begin() self.assertEqual(resp.read(), expected) # the file should now have our extradata ready to be read - self.assertEqual(sock.file.read(100), extradata.encode("ascii")) #we read to the end + self.assertEqual(sock.file.read(), extradata.encode("ascii")) #we read to the end resp.close() def test_content_length_sync(self): """Check that we don't read past the end of the Content-Length stream""" - extradata = "extradata" + extradata = b"extradata" + expected = b"Hello123\r\n" + sock = FakeSocket(b'HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\n' + expected + extradata) + resp = client.HTTPResponse(sock, method="GET") + resp.begin() + self.assertEqual(resp.read(), expected) + # the file should now have our extradata ready to be read + self.assertEqual(sock.file.read(), extradata) #we read to the end + resp.close() + + def test_readlines_content_length(self): + extradata = b"extradata" + expected = b"Hello123\r\n" + sock = FakeSocket(b'HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\n' + expected + extradata) + resp = client.HTTPResponse(sock, method="GET") + resp.begin() + self.assertEqual(resp.readlines(2000), [expected]) + # the file should now have our extradata ready to be read + self.assertEqual(sock.file.read(), extradata) #we read to the end + resp.close() + + def test_read1_content_length(self): + extradata = b"extradata" + expected = b"Hello123\r\n" + sock = FakeSocket(b'HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\n' + expected + extradata) + resp = client.HTTPResponse(sock, method="GET") + resp.begin() + self.assertEqual(resp.read1(2000), expected) + # the file should now have our extradata ready to be read + self.assertEqual(sock.file.read(), extradata) #we read to the end + resp.close() + + def test_readline_bound_content_length(self): + extradata = b"extradata" + expected = b"Hello123\r\n" + sock = FakeSocket(b'HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\n' + expected + extradata) + resp = client.HTTPResponse(sock, method="GET") + resp.begin() + self.assertEqual(resp.readline(10), expected) + self.assertEqual(resp.readline(10), b"") + # the file should now have our extradata ready to be read + self.assertEqual(sock.file.read(), extradata) #we read to the end + resp.close() + + def test_read1_bound_content_length(self): + extradata = b"extradata" expected = b"Hello123\r\n" - sock = FakeSocket('HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\nHello123\r\n' + extradata) + sock = FakeSocket(b'HTTP/1.1 200 OK\r\nContent-Length: 30\r\n\r\n' + expected*3 + extradata) resp = client.HTTPResponse(sock, method="GET") resp.begin() + self.assertEqual(resp.read1(20), expected*2) self.assertEqual(resp.read(), expected) # the file should now have our extradata ready to be read - self.assertEqual(sock.file.read(100), extradata.encode("ascii")) #we read to the end + self.assertEqual(sock.file.read(), extradata) #we read to the end resp.close() class ExtendedReadTest(TestCase): diff --git a/Misc/NEWS b/Misc/NEWS index 9cb7f8c..7516932 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -91,6 +91,10 @@ Core and Builtins Library ------- +- Issue #26499: Account for remaining Content-Length in + HTTPResponse.readline() and read1(). Based on patch by Silent Ghost. + Also document that HTTPResponse now supports these methods. + - Issue #25320: Handle sockets in directories unittest discovery is scanning. Patch from Victor van den Elzen. -- cgit v0.12