Skip to content

Commit f86064e

Browse files
[3.11] gh-150743: Limit trailer lines and interim responses read by http.client (GH-150749) (#152528)
1 parent 56b7100 commit f86064e

3 files changed

Lines changed: 84 additions & 1 deletion

File tree

Lib/http/client.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,13 @@
111111
_MAXLINE = 65536
112112
_MAXHEADERS = 100
113113

114+
# maximal number of interim (1xx) responses tolerated before the final
115+
# response. Real servers send at most a few; without a bound, a server
116+
# streaming "100 Continue" responses would hang getresponse() forever.
117+
# A socket timeout cannot detect that as data keeps arriving within every
118+
# timeout window.
119+
_MAXINTERIMRESPONSES = 100
120+
114121
# Data larger than this will be read in chunks, to prevent extreme
115122
# overallocation.
116123
_MIN_READ_BUF_SIZE = 1 << 20
@@ -326,7 +333,7 @@ def begin(self):
326333
return
327334

328335
# read until we get a non-100 response
329-
while True:
336+
for _ in range(_MAXINTERIMRESPONSES):
330337
version, status, reason = self._read_status()
331338
if status != CONTINUE:
332339
break
@@ -335,6 +342,9 @@ def begin(self):
335342
if self.debuglevel > 0:
336343
print("headers:", skipped_headers)
337344
del skipped_headers
345+
else:
346+
raise HTTPException(
347+
f"got more than {_MAXINTERIMRESPONSES} interim responses")
338348

339349
self.code = self.status = status
340350
self.reason = reason.strip()
@@ -552,6 +562,7 @@ def _read_next_chunk_size(self):
552562
def _read_and_discard_trailer(self):
553563
# read and discard trailer up to the CRLF terminator
554564
### note: we shouldn't have any trailers!
565+
trailers_read = 0
555566
while True:
556567
line = self.fp.readline(_MAXLINE + 1)
557568
if len(line) > _MAXLINE:
@@ -562,6 +573,14 @@ def _read_and_discard_trailer(self):
562573
break
563574
if line in (b'\r\n', b'\n', b''):
564575
break
576+
# Bound the trailer count just as response headers are bounded.
577+
# A server streaming trailer lines forever would otherwise hang
578+
# the client; a socket timeout cannot detect that as data keeps
579+
# arriving within every timeout window.
580+
trailers_read += 1
581+
if trailers_read > _MAXHEADERS:
582+
raise HTTPException(
583+
f"got more than {_MAXHEADERS} trailers")
565584

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

Lib/test/test_httplib.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,6 +1257,35 @@ def test_overflowing_header_limit_after_100(self):
12571257
self.assertIn('got more than ', str(cm.exception))
12581258
self.assertIn('headers', str(cm.exception))
12591259

1260+
def test_too_many_interim_responses(self):
1261+
# A server streaming "100 Continue" responses forever must not
1262+
# hang getresponse().
1263+
body = (
1264+
'HTTP/1.1 100 Continue\r\n\r\n'
1265+
* (client._MAXINTERIMRESPONSES + 1)
1266+
)
1267+
resp = client.HTTPResponse(FakeSocket(body))
1268+
with self.assertRaises(client.HTTPException) as cm:
1269+
resp.begin()
1270+
self.assertIn('got more than ', str(cm.exception))
1271+
self.assertIn('interim responses', str(cm.exception))
1272+
1273+
def test_multiple_interim_responses(self):
1274+
# A reasonable number of interim responses before the final
1275+
# response is skipped as before.
1276+
body = (
1277+
'HTTP/1.1 100 Continue\r\n\r\n' * 3 +
1278+
'HTTP/1.1 200 OK\r\n'
1279+
'Content-Length: 5\r\n'
1280+
'\r\n'
1281+
'hello'
1282+
)
1283+
resp = client.HTTPResponse(FakeSocket(body), method="GET")
1284+
resp.begin()
1285+
self.assertEqual(resp.status, 200)
1286+
self.assertEqual(resp.read(), b'hello')
1287+
resp.close()
1288+
12601289
def test_overflowing_chunked_line(self):
12611290
body = (
12621291
'HTTP/1.1 200 OK\r\n'
@@ -1328,6 +1357,35 @@ def test_chunked_trailers(self):
13281357
self.assertEqual(sock.file.read(), b"") #we read to the end
13291358
resp.close()
13301359

1360+
def test_chunked_too_many_trailers(self):
1361+
"""A response streaming endless trailer lines must raise, not hang"""
1362+
too_many_trailers = "".join(
1363+
f"X-Trailer{i}: {i}\r\n" for i in range(client._MAXHEADERS + 1)
1364+
)
1365+
# An unbounded read() reaches the trailers via the final 0 chunk.
1366+
sock = FakeSocket(
1367+
chunked_start + last_chunk + too_many_trailers + chunked_end)
1368+
resp = client.HTTPResponse(sock, method="GET")
1369+
resp.begin()
1370+
with self.assertRaisesRegex(
1371+
client.HTTPException,
1372+
f"got more than {client._MAXHEADERS} trailers",
1373+
):
1374+
resp.read()
1375+
resp.close()
1376+
1377+
# A bounded read(amt) larger than the body hits the same limit.
1378+
sock = FakeSocket(
1379+
chunked_start + last_chunk + too_many_trailers + chunked_end)
1380+
resp = client.HTTPResponse(sock, method="GET")
1381+
resp.begin()
1382+
with self.assertRaisesRegex(
1383+
client.HTTPException,
1384+
f"got more than {client._MAXHEADERS} trailers",
1385+
):
1386+
resp.read(len(chunked_expected) + 1)
1387+
resp.close()
1388+
13311389
def test_chunked_sync(self):
13321390
"""Check that we don't read past the end of the chunked-encoding stream"""
13331391
expected = chunked_expected
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
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.
6+
Reported by ``@YLChen-007`` via GHSA-w4q2-g22w-6fr4.

0 commit comments

Comments
 (0)