Skip to content

Commit 63b37eb

Browse files
committed
http.client: bound the number of chunked trailer lines read
A server could stream syntactically valid trailer lines forever after the final chunk of a chunked response, so reading the response would never return. Socket timeouts cannot interrupt this because data keeps arriving within every timeout window. Trailer lines are now counted against the same limit as response headers (100 by default) and HTTPException is raised when the limit is exceeded. (cherry picked from commit 752bef5047e54e21af0e48a339b54fbae2a6c831)
1 parent 13bb7f7 commit 63b37eb

3 files changed

Lines changed: 43 additions & 0 deletions

File tree

Lib/http/client.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,7 @@ def _read_next_chunk_size(self):
558558
def _read_and_discard_trailer(self):
559559
# read and discard trailer up to the CRLF terminator
560560
### note: we shouldn't have any trailers!
561+
trailers_read = 0
561562
while True:
562563
line = self.fp.readline(_MAXLINE + 1)
563564
if len(line) > _MAXLINE:
@@ -568,6 +569,14 @@ def _read_and_discard_trailer(self):
568569
break
569570
if line in (b'\r\n', b'\n', b''):
570571
break
572+
# Bound the trailer count just as response headers are bounded.
573+
# A server streaming trailer lines forever would otherwise hang
574+
# the client; a socket timeout cannot detect that as data keeps
575+
# arriving within every timeout window.
576+
trailers_read += 1
577+
if trailers_read > _MAXHEADERS:
578+
raise HTTPException(
579+
f"got more than {_MAXHEADERS} trailers")
571580

572581
def _get_chunk_left(self):
573582
# return self.chunk_left, reading a new chunk if necessary.

Lib/test/test_httplib.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1403,6 +1403,35 @@ def test_chunked_trailers(self):
14031403
self.assertEqual(sock.file.read(), b"") #we read to the end
14041404
resp.close()
14051405

1406+
def test_chunked_too_many_trailers(self):
1407+
"""A response streaming endless trailer lines must raise, not hang"""
1408+
too_many_trailers = "".join(
1409+
f"X-Trailer{i}: {i}\r\n" for i in range(client._MAXHEADERS + 1)
1410+
)
1411+
# An unbounded read() reaches the trailers via the final 0 chunk.
1412+
sock = FakeSocket(
1413+
chunked_start + last_chunk + too_many_trailers + chunked_end)
1414+
resp = client.HTTPResponse(sock, method="GET")
1415+
resp.begin()
1416+
with self.assertRaisesRegex(
1417+
client.HTTPException,
1418+
f"got more than {client._MAXHEADERS} trailers",
1419+
):
1420+
resp.read()
1421+
resp.close()
1422+
1423+
# A bounded read(amt) larger than the body hits the same limit.
1424+
sock = FakeSocket(
1425+
chunked_start + last_chunk + too_many_trailers + chunked_end)
1426+
resp = client.HTTPResponse(sock, method="GET")
1427+
resp.begin()
1428+
with self.assertRaisesRegex(
1429+
client.HTTPException,
1430+
f"got more than {client._MAXHEADERS} trailers",
1431+
):
1432+
resp.read(len(chunked_expected) + 1)
1433+
resp.close()
1434+
14061435
def test_chunked_sync(self):
14071436
"""Check that we don't read past the end of the chunked-encoding stream"""
14081437
expected = chunked_expected
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
:mod:`http.client` now limits the number of chunked-response trailer lines
2+
it will read to 100, and the number of interim (1xx) responses it will
3+
skip to 100. A malicious or broken server could previously stream trailer
4+
lines or ``100 Continue`` responses forever, hanging the client even when
5+
a socket timeout was in use.

0 commit comments

Comments
 (0)