Issue #26499: Fixes to HTTPResponse.readline() and read1(), by Silent Ghost
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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('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: 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(100), extradata.encode("ascii")) #we read to the end
+ 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(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(), 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 @@
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.