Visitar URL original
http1connection: Return after reading the response that follows a 1xx · tornadoweb/tornado@b798f83 · GitHub
Skip to content

Commit b798f83

Browse files
claudeclaude
committed
http1connection: Return after reading the response that follows a 1xx
The 1xx branch of _read_message recursed to read the real response but did not return afterward, so the outer frame fell through and ran the rest of the function a second time. With a single "100 Continue" the delegate was finished twice and a second body was read; with three, the delegate was finished four times and _read_body ran four times. A 1xx response has no Content-Length and is not chunked, so each of those extra _read_body calls landed in _read_body_until_close. That was survivable only when the server closed the connection promptly. Against a server that keeps the connection open, the outer frame is left blocked on a read that will never be satisfied and read_response never completes. Return the recursive call's result directly. The recursive call owns the delegate from that point, so need_delegate_close is cleared first to keep the finally block from calling on_connection_close() on a delegate that has already been finished. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JD7LUuFMdyPJB1Dwdq6fPg
1 parent 2670ca8 commit b798f83

2 files changed

Lines changed: 42 additions & 1 deletion

File tree

‎tornado/http1connection.py‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,15 @@ async def _read_message(
265265
raise httputil.HTTPInputError("Too many 1xx responses")
266266
# TODO: client delegates will get headers_received twice
267267
# in the case of a 100-continue. Document or change?
268-
await self._read_message(delegate, num_1xx + 1)
268+
#
269+
# The recursive call reads the real response and owns
270+
# the delegate from here on, so there is nothing left
271+
# for this frame to do. Clear need_delegate_close so
272+
# that the finally block does not call
273+
# on_connection_close() on an already-finished
274+
# delegate.
275+
need_delegate_close = False
276+
return await self._read_message(delegate, num_1xx + 1)
269277
else:
270278
if headers.get("Expect") == "100-continue" and not self._write_finished:
271279
self.stream.write(b"HTTP/1.1 100 (Continue)\r\n\r\n")

‎tornado/test/http1connection_test.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,39 @@ def accept_callback(conn, addr):
3333
self.io_loop.remove_handler(listener)
3434
listener.close()
3535

36+
@gen_test
37+
def test_1xx_does_not_read_a_second_body(self):
38+
# An informational (1xx) response is followed by the real response,
39+
# which is read by a recursive call to _read_message. Once that
40+
# returns, the outer call has nothing left to do: it must not fall
41+
# through and read a second body, which would finish the delegate
42+
# a second time and start a spurious read-until-close.
43+
conn = HTTP1Connection(self.client_stream, True)
44+
self.server_stream.write(b"HTTP/1.1 100 CONTINUE\r\n\r\n")
45+
self.server_stream.write(b"HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\nhello")
46+
self.server_stream.close()
47+
48+
body = []
49+
finish_count = []
50+
close_count = []
51+
52+
class Delegate(HTTPMessageDelegate):
53+
def data_received(self, data):
54+
body.append(data)
55+
56+
def finish(self):
57+
finish_count.append(1)
58+
59+
def on_connection_close(self):
60+
close_count.append(1)
61+
62+
yield conn.read_response(Delegate())
63+
self.assertEqual(b"".join(body), b"hello")
64+
self.assertEqual(len(finish_count), 1)
65+
# The delegate finished normally, so it must not also be told the
66+
# connection closed on it.
67+
self.assertEqual(len(close_count), 0)
68+
3669
@gen_test
3770
def test_http10_no_content_length(self):
3871
# Regression test for a bug in which can_keep_alive would crash

0 commit comments

Comments
 (0)