From 2eac71e14b597d38888aba2ce3b0edbf49603d41 Mon Sep 17 00:00:00 2001 From: Andres Rivero Date: Thu, 10 Sep 2026 21:34:11 -0700 Subject: [PATCH 1/6] fix: merge duplicate Transfer-Encoding: chunked response headers Some servers send a redundant, byte-identical Transfer-Encoding: chunked header line twice on the wire, which h11 correctly rejects per RFC 9112 but which requests and browsers tolerate. Normalize this narrow, safe case the same way h11 already tolerates duplicate identical Content-Length headers, before the bytes ever reach h11's parser. Anything else (differing values, other conflicts) still raises RemoteProtocolError exactly as before. Fixes #622 --- src/httpcore2/httpcore2/_async/http11.py | 86 ++++++++++++++++++++- src/httpcore2/httpcore2/_sync/http11.py | 86 ++++++++++++++++++++- tests/httpcore2/_async/test_http11.py | 96 ++++++++++++++++++++++++ tests/httpcore2/_sync/test_http11.py | 96 ++++++++++++++++++++++++ 4 files changed, 360 insertions(+), 4 deletions(-) diff --git a/src/httpcore2/httpcore2/_async/http11.py b/src/httpcore2/httpcore2/_async/http11.py index 2fc94452..04b03438 100644 --- a/src/httpcore2/httpcore2/_async/http11.py +++ b/src/httpcore2/httpcore2/_async/http11.py @@ -38,6 +38,87 @@ class HTTPConnectionState(enum.IntEnum): CLOSED = 3 +def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: + """ + Merge exact-duplicate `Transfer-Encoding: chunked` header lines into one. + + Some servers send this redundant duplicate on the wire (see + https://github.com/pydantic/httpx2/issues/622). h11 already tolerates + duplicate `Content-Length` headers so long as every value is identical; + this mirrors that same narrow tolerance for `Transfer-Encoding`. Any + other case (differing values, non-`chunked` duplicates) is left + untouched, so h11 still raises for it exactly as before. + """ + status_line, *header_lines = header_block.split(b"\r\n") + + seen_chunked_transfer_encoding = False + merged_lines = [] + for line in header_lines: + name, sep, value = line.partition(b":") + if sep and name.strip().lower() == b"transfer-encoding" and value.strip().lower() == b"chunked": + if seen_chunked_transfer_encoding: + continue + seen_chunked_transfer_encoding = True + merged_lines.append(line) + + return b"\r\n".join([status_line, *merged_lines]) + + +class AsyncHTTP11ResponseNormalizingStream(AsyncNetworkStream): + """ + Wraps the underlying network stream and, while a response's headers are + still being received, normalizes them via `_merge_duplicate_chunked_transfer_encoding` + before h11 ever sees the bytes. Once the header block is found (or + `max_buffer_size` is exceeded without finding it), reads pass straight + through unmodified for the rest of that response cycle. + """ + + def __init__(self, stream: AsyncNetworkStream, max_buffer_size: int) -> None: + self._stream = stream + self._max_buffer_size = max_buffer_size + self._buffer: bytes | None = b"" + + def reset(self) -> None: + self._buffer = b"" + + async def read(self, max_bytes: int, timeout: float | None = None) -> bytes: + if self._buffer is None: + return await self._stream.read(max_bytes, timeout) + + while True: + chunk = await self._stream.read(max_bytes, timeout) + if not chunk: + return chunk + + self._buffer += chunk + header_block, separator, rest = self._buffer.partition(b"\r\n\r\n") + if separator: + self._buffer = None + return _merge_duplicate_chunked_transfer_encoding(header_block) + separator + rest + + if len(self._buffer) > self._max_buffer_size: + buffered = self._buffer + self._buffer = None + return buffered + + async def write(self, buffer: bytes, timeout: float | None = None) -> None: + await self._stream.write(buffer, timeout) + + async def aclose(self) -> None: + await self._stream.aclose() + + async def start_tls( + self, + ssl_context: ssl.SSLContext, + server_hostname: str | None = None, + timeout: float | None = None, + ) -> AsyncNetworkStream: + return await self._stream.start_tls(ssl_context, server_hostname, timeout) + + def get_extra_info(self, info: str) -> typing.Any: + return self._stream.get_extra_info(info) + + class AsyncHTTP11Connection(AsyncConnectionInterface): READ_NUM_BYTES = 64 * 1024 MAX_INCOMPLETE_EVENT_SIZE = 100 * 1024 @@ -49,7 +130,7 @@ def __init__( keepalive_expiry: float | None = None, ) -> None: self._origin = origin - self._network_stream = stream + self._network_stream = AsyncHTTP11ResponseNormalizingStream(stream, self.MAX_INCOMPLETE_EVENT_SIZE) self._keepalive_expiry: float | None = keepalive_expiry self._expire_at: float | None = None self._state = HTTPConnectionState.NEW @@ -102,7 +183,7 @@ async def handle_async_request(self, request: Request) -> Response: headers, ) - network_stream = self._network_stream + network_stream: AsyncNetworkStream = self._network_stream # CONNECT or Upgrade request if (status == 101) or ((request.method == b"CONNECT") and (200 <= status < 300)): @@ -221,6 +302,7 @@ async def _response_closed(self) -> None: if self._h11_state.our_state is h11.DONE and self._h11_state.their_state is h11.DONE: self._state = HTTPConnectionState.IDLE self._h11_state.start_next_cycle() + self._network_stream.reset() if self._keepalive_expiry is not None: now = time.monotonic() self._expire_at = now + self._keepalive_expiry diff --git a/src/httpcore2/httpcore2/_sync/http11.py b/src/httpcore2/httpcore2/_sync/http11.py index 50bce833..43561374 100644 --- a/src/httpcore2/httpcore2/_sync/http11.py +++ b/src/httpcore2/httpcore2/_sync/http11.py @@ -38,6 +38,87 @@ class HTTPConnectionState(enum.IntEnum): CLOSED = 3 +def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: + """ + Merge exact-duplicate `Transfer-Encoding: chunked` header lines into one. + + Some servers send this redundant duplicate on the wire (see + https://github.com/pydantic/httpx2/issues/622). h11 already tolerates + duplicate `Content-Length` headers so long as every value is identical; + this mirrors that same narrow tolerance for `Transfer-Encoding`. Any + other case (differing values, non-`chunked` duplicates) is left + untouched, so h11 still raises for it exactly as before. + """ + status_line, *header_lines = header_block.split(b"\r\n") + + seen_chunked_transfer_encoding = False + merged_lines = [] + for line in header_lines: + name, sep, value = line.partition(b":") + if sep and name.strip().lower() == b"transfer-encoding" and value.strip().lower() == b"chunked": + if seen_chunked_transfer_encoding: + continue + seen_chunked_transfer_encoding = True + merged_lines.append(line) + + return b"\r\n".join([status_line, *merged_lines]) + + +class HTTP11ResponseNormalizingStream(NetworkStream): + """ + Wraps the underlying network stream and, while a response's headers are + still being received, normalizes them via `_merge_duplicate_chunked_transfer_encoding` + before h11 ever sees the bytes. Once the header block is found (or + `max_buffer_size` is exceeded without finding it), reads pass straight + through unmodified for the rest of that response cycle. + """ + + def __init__(self, stream: NetworkStream, max_buffer_size: int) -> None: + self._stream = stream + self._max_buffer_size = max_buffer_size + self._buffer: bytes | None = b"" + + def reset(self) -> None: + self._buffer = b"" + + def read(self, max_bytes: int, timeout: float | None = None) -> bytes: + if self._buffer is None: + return self._stream.read(max_bytes, timeout) + + while True: + chunk = self._stream.read(max_bytes, timeout) + if not chunk: + return chunk + + self._buffer += chunk + header_block, separator, rest = self._buffer.partition(b"\r\n\r\n") + if separator: + self._buffer = None + return _merge_duplicate_chunked_transfer_encoding(header_block) + separator + rest + + if len(self._buffer) > self._max_buffer_size: + buffered = self._buffer + self._buffer = None + return buffered + + def write(self, buffer: bytes, timeout: float | None = None) -> None: + self._stream.write(buffer, timeout) + + def close(self) -> None: + self._stream.close() + + def start_tls( + self, + ssl_context: ssl.SSLContext, + server_hostname: str | None = None, + timeout: float | None = None, + ) -> NetworkStream: + return self._stream.start_tls(ssl_context, server_hostname, timeout) + + def get_extra_info(self, info: str) -> typing.Any: + return self._stream.get_extra_info(info) + + class HTTP11Connection(ConnectionInterface): READ_NUM_BYTES = 64 * 1024 MAX_INCOMPLETE_EVENT_SIZE = 100 * 1024 @@ -49,7 +130,7 @@ def __init__( keepalive_expiry: float | None = None, ) -> None: self._origin = origin - self._network_stream = stream + self._network_stream = HTTP11ResponseNormalizingStream(stream, self.MAX_INCOMPLETE_EVENT_SIZE) self._keepalive_expiry: float | None = keepalive_expiry self._expire_at: float | None = None self._state = HTTPConnectionState.NEW @@ -102,7 +183,7 @@ def handle_request(self, request: Request) -> Response: headers, ) - network_stream = self._network_stream + network_stream: NetworkStream = self._network_stream # CONNECT or Upgrade request if (status == 101) or ((request.method == b"CONNECT") and (200 <= status < 300)): @@ -221,6 +302,7 @@ def _response_closed(self) -> None: if self._h11_state.our_state is h11.DONE and self._h11_state.their_state is h11.DONE: self._state = HTTPConnectionState.IDLE self._h11_state.start_next_cycle() + self._network_stream.reset() if self._keepalive_expiry is not None: now = time.monotonic() self._expire_at = now + self._keepalive_expiry diff --git a/tests/httpcore2/_async/test_http11.py b/tests/httpcore2/_async/test_http11.py index d0087b19..8c3e6736 100644 --- a/tests/httpcore2/_async/test_http11.py +++ b/tests/httpcore2/_async/test_http11.py @@ -326,6 +326,102 @@ async def test_http11_early_hints() -> None: assert response.content == b"Hello, world! ..." +@pytest.mark.anyio +async def test_http11_connection_merges_duplicate_chunked_transfer_encoding() -> None: + """ + Some servers send `Transfer-Encoding: chunked` twice on the wire (e.g. + https://github.com/pydantic/httpx2/issues/622). Duplicate, byte-identical + `Transfer-Encoding: chunked` header lines should be merged into one, + mirroring how h11 already tolerates duplicate identical Content-Length + headers, rather than raising `RemoteProtocolError`. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.AsyncMockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"Content-Type: text/plain\r\n", + b"Transfer-Encoding: chunked\r\n", + b"Transfer-Encoding: chunked\r\n", + b"\r\n", + b"5\r\nHello\r\n0\r\n\r\n", + ] + ) + async with httpcore2.AsyncHTTP11Connection(origin=origin, stream=stream) as conn: + response = await conn.request("GET", "https://example.com/") + assert response.status == 200 + assert response.content == b"Hello" + + transfer_encodings = [v for k, v in response.headers if k.lower() == b"transfer-encoding"] + assert transfer_encodings == [b"chunked"] + + +@pytest.mark.anyio +async def test_http11_connection_merges_duplicate_chunked_transfer_encoding_split_across_reads() -> None: + """ + The merge must work even when the duplicate header line, and the + terminating blank line, are split across separate network reads. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.AsyncMockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"Content-Type: text/plain\r\n", + b"Transfer-Encoding: chunked\r\nTransfer-Enco", + b"ding: chunked\r\n\r\n", + b"5\r\nHello\r\n0\r\n\r\n", + ] + ) + async with httpcore2.AsyncHTTP11Connection(origin=origin, stream=stream) as conn: + response = await conn.request("GET", "https://example.com/") + assert response.status == 200 + assert response.content == b"Hello" + + transfer_encodings = [v for k, v in response.headers if k.lower() == b"transfer-encoding"] + assert transfer_encodings == [b"chunked"] + + +@pytest.mark.anyio +async def test_http11_connection_with_conflicting_transfer_encoding_headers() -> None: + """ + Duplicate `Transfer-Encoding` headers with *differing* values are not a + safe, unambiguous case, so they should still raise `RemoteProtocolError` + exactly as before. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.AsyncMockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"Transfer-Encoding: chunked\r\n", + b"Transfer-Encoding: identity\r\n", + b"\r\n", + b"", + ] + ) + async with httpcore2.AsyncHTTP11Connection(origin=origin, stream=stream) as conn: + with pytest.raises(httpcore2.RemoteProtocolError): + await conn.request("GET", "https://example.com/") + + +@pytest.mark.anyio +async def test_http11_connection_with_oversized_headers_and_no_terminator() -> None: + """ + If the header block never terminates and grows past the incomplete-event + size bound, we must still hand off to h11 (which enforces its own limit) + rather than buffering unboundedly. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.AsyncMockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"Cookie: " + b"x" * (100 * 1024) + b"\r\n", + b"", + ] + ) + async with httpcore2.AsyncHTTP11Connection(origin=origin, stream=stream) as conn: + with pytest.raises(httpcore2.RemoteProtocolError): + await conn.request("GET", "https://example.com/") + + @pytest.mark.anyio async def test_http11_header_sub_100kb() -> None: """ diff --git a/tests/httpcore2/_sync/test_http11.py b/tests/httpcore2/_sync/test_http11.py index f886ef47..9e2cde67 100644 --- a/tests/httpcore2/_sync/test_http11.py +++ b/tests/httpcore2/_sync/test_http11.py @@ -327,6 +327,102 @@ def test_http11_early_hints() -> None: +def test_http11_connection_merges_duplicate_chunked_transfer_encoding() -> None: + """ + Some servers send `Transfer-Encoding: chunked` twice on the wire (e.g. + https://github.com/pydantic/httpx2/issues/622). Duplicate, byte-identical + `Transfer-Encoding: chunked` header lines should be merged into one, + mirroring how h11 already tolerates duplicate identical Content-Length + headers, rather than raising `RemoteProtocolError`. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.MockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"Content-Type: text/plain\r\n", + b"Transfer-Encoding: chunked\r\n", + b"Transfer-Encoding: chunked\r\n", + b"\r\n", + b"5\r\nHello\r\n0\r\n\r\n", + ] + ) + with httpcore2.HTTP11Connection(origin=origin, stream=stream) as conn: + response = conn.request("GET", "https://example.com/") + assert response.status == 200 + assert response.content == b"Hello" + + transfer_encodings = [v for k, v in response.headers if k.lower() == b"transfer-encoding"] + assert transfer_encodings == [b"chunked"] + + + +def test_http11_connection_merges_duplicate_chunked_transfer_encoding_split_across_reads() -> None: + """ + The merge must work even when the duplicate header line, and the + terminating blank line, are split across separate network reads. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.MockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"Content-Type: text/plain\r\n", + b"Transfer-Encoding: chunked\r\nTransfer-Enco", + b"ding: chunked\r\n\r\n", + b"5\r\nHello\r\n0\r\n\r\n", + ] + ) + with httpcore2.HTTP11Connection(origin=origin, stream=stream) as conn: + response = conn.request("GET", "https://example.com/") + assert response.status == 200 + assert response.content == b"Hello" + + transfer_encodings = [v for k, v in response.headers if k.lower() == b"transfer-encoding"] + assert transfer_encodings == [b"chunked"] + + + +def test_http11_connection_with_conflicting_transfer_encoding_headers() -> None: + """ + Duplicate `Transfer-Encoding` headers with *differing* values are not a + safe, unambiguous case, so they should still raise `RemoteProtocolError` + exactly as before. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.MockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"Transfer-Encoding: chunked\r\n", + b"Transfer-Encoding: identity\r\n", + b"\r\n", + b"", + ] + ) + with httpcore2.HTTP11Connection(origin=origin, stream=stream) as conn: + with pytest.raises(httpcore2.RemoteProtocolError): + conn.request("GET", "https://example.com/") + + + +def test_http11_connection_with_oversized_headers_and_no_terminator() -> None: + """ + If the header block never terminates and grows past the incomplete-event + size bound, we must still hand off to h11 (which enforces its own limit) + rather than buffering unboundedly. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.MockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"Cookie: " + b"x" * (100 * 1024) + b"\r\n", + b"", + ] + ) + with httpcore2.HTTP11Connection(origin=origin, stream=stream) as conn: + with pytest.raises(httpcore2.RemoteProtocolError): + conn.request("GET", "https://example.com/") + + + def test_http11_header_sub_100kb() -> None: """ A connection should be able to handle a http header size up to 100kB. From 479f0c89fe20d2921b645970ac145b8b9166a764 Mon Sep 17 00:00:00 2001 From: Andres Rivero Date: Fri, 11 Sep 2026 18:28:28 -0700 Subject: [PATCH 2/6] fix: normalize Transfer-Encoding duplicates via h11's own parse state The previous commit normalized duplicate Transfer-Encoding headers with a standalone byte-stream wrapper that re-implemented HTTP header parsing ahead of h11, using its own \r\n\r\n boundary and \r\n line-splitting assumptions. Two independent security reviews found this unsound: - h11 tolerates a bare \n (not just \r\n\r\n) as a header terminator and splits lines on \n, so a non-\r\n header block let normalization run past the real boundary into response body bytes, corrupting them and shifting message framing onto the next pooled response. - h11 supports obsolete line folding (a continuation line starting with whitespace); the wrapper's case/whitespace-insensitive matching could treat a folded continuation as a standalone duplicate and delete it, corrupting an unrelated header's value and, in one reproduction, fully desyncing a pooled connection so the next request received a response the server never sent for it. - The wrapper also silently stopped applying after any 1xx interim response (100 Continue / 103 Early Hints) ahead of the final response, and a case/whitespace-insensitive match let malformed duplicate lines bypass h11's own validation. This replaces that wrapper with normalization gated directly on `h11_state.their_state == h11.SEND_RESPONSE`, mirroring h11's actual boundary regex and line-splitting so a header block is parsed exactly the way h11 will parse it, and re-arming naturally across 1xx responses via h11's own state machine instead of manual bookkeeping. Only ever removes a line that is unfolded, not the status line, not itself followed by a fold continuation, and matches (case-insensitive name, OWS-stripped-and-lowered value) exactly `(transfer-encoding, chunked)` after an identical earlier line -- everything else is left untouched so h11 still raises for it exactly as before. A second review round of this replacement found one more real issue: the boundary search only looked inside this connection's own accumulator, not bytes h11 might already be holding unconsumed from a previous read (e.g. a pipelined response, or two responses landing in the same TCP read on a keep-alive connection -- an everyday occurrence, not an edge case). When the true boundary straddled that hidden junction, the search could lock onto a later, coincidental match inside the body. Fixed by skipping normalization whenever h11 already holds unparsed trailing data at the point a new header block would start, falling back to h11's pre-existing (safe) handling for that read. Both rounds' proof-of-concept payloads, plus the original issue-622 reproduction over a real TCP socket, are verified fixed. Full suite (2020 tests) passes with 100% coverage; mypy strict and ruff are clean. Fixes #622 --- src/httpcore2/httpcore2/_async/http11.py | 247 +++++++++++++++-------- src/httpcore2/httpcore2/_sync/http11.py | 247 +++++++++++++++-------- tests/httpcore2/_async/test_http11.py | 142 +++++++++++++ tests/httpcore2/_sync/test_http11.py | 142 +++++++++++++ 4 files changed, 604 insertions(+), 174 deletions(-) diff --git a/src/httpcore2/httpcore2/_async/http11.py b/src/httpcore2/httpcore2/_async/http11.py index 04b03438..39cf69c4 100644 --- a/src/httpcore2/httpcore2/_async/http11.py +++ b/src/httpcore2/httpcore2/_async/http11.py @@ -2,6 +2,7 @@ import enum import logging +import re import ssl import time import types @@ -38,85 +39,87 @@ class HTTPConnectionState(enum.IntEnum): CLOSED = 3 -def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: - """ - Merge exact-duplicate `Transfer-Encoding: chunked` header lines into one. - - Some servers send this redundant duplicate on the wire (see - https://github.com/pydantic/httpx2/issues/622). h11 already tolerates - duplicate `Content-Length` headers so long as every value is identical; - this mirrors that same narrow tolerance for `Transfer-Encoding`. Any - other case (differing values, non-`chunked` duplicates) is left - untouched, so h11 still raises for it exactly as before. - """ - status_line, *header_lines = header_block.split(b"\r\n") - - seen_chunked_transfer_encoding = False - merged_lines = [] - for line in header_lines: - name, sep, value = line.partition(b":") - if sep and name.strip().lower() == b"transfer-encoding" and value.strip().lower() == b"chunked": - if seen_chunked_transfer_encoding: - continue - seen_chunked_transfer_encoding = True - merged_lines.append(line) +# Mirrors h11's own header/body boundary (`h11._receivebuffer.blank_line_regex`): +# h11 tolerates a bare `\n` or `\n\r\n`, not just `\r\n\r\n`. +_HEADER_BLOCK_TERMINATOR_RE = re.compile(rb"\n\r?\n") - return b"\r\n".join([status_line, *merged_lines]) - -class AsyncHTTP11ResponseNormalizingStream(AsyncNetworkStream): +def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: """ - Wraps the underlying network stream and, while a response's headers are - still being received, normalizes them via `_merge_duplicate_chunked_transfer_encoding` - before h11 ever sees the bytes. Once the header block is found (or - `max_buffer_size` is exceeded without finding it), reads pass straight - through unmodified for the rest of that response cycle. + Merge an exact-duplicate `Transfer-Encoding: chunked` header line into an + earlier one, mirroring h11's existing tolerance for duplicate identical + Content-Length headers (see https://github.com/pydantic/httpx2/issues/622). + + `header_block` must end with the header/body boundary matched by + `_HEADER_BLOCK_TERMINATOR_RE`, boundary bytes included. Lines are split + the same way h11 splits them (on `\n`, with one optional trailing `\r` + stripped per line -- see `h11._receivebuffer.ReceiveBuffer.maybe_extract_lines`) + so a header block using non-`\r\n` line endings is parsed identically to + how h11 will parse it. + + Only ever *removes* bytes that are provably an exact, unfolded repeat of + an earlier `Transfer-Encoding: chunked` line: + + - Header names are matched case-insensitively (legitimate per RFC 9110), + but never stripped of surrounding whitespace -- a real header field has + no whitespace between the name and the colon, so anything like + `Transfer-Encoding : chunked` fails to match and is left for h11 to + reject as an illegal header line. + - A candidate line is skipped entirely if it starts with a fold-indicating + space/tab (RFC 9112 obsolete line folding: it's a continuation of the + *previous* header's value, not a standalone header) or if the following + line does -- in the latter case deleting it would orphan that + continuation, changing which header it folds into. + + Any other case (differing values, folded lines, malformed lines) is left + completely untouched, so h11 still raises for it exactly as before. """ + line_spans: list[tuple[bytes, int, int]] = [] + start = 0 + for match in re.finditer(rb"\n", header_block): + end = match.end() + content_end = match.start() + if header_block[content_end - 1 : content_end] == b"\r": + content_end -= 1 + line_spans.append((header_block[start:content_end], start, end)) + start = end + + # The final span is always the second half of the header/body boundary + # itself (mirroring h11's own `del lines[-2:]`), never a real header line. + header_line_spans = line_spans[:-1] - def __init__(self, stream: AsyncNetworkStream, max_buffer_size: int) -> None: - self._stream = stream - self._max_buffer_size = max_buffer_size - self._buffer: bytes | None = b"" - - def reset(self) -> None: - self._buffer = b"" - - async def read(self, max_bytes: int, timeout: float | None = None) -> bytes: - if self._buffer is None: - return await self._stream.read(max_bytes, timeout) + seen_chunked_transfer_encoding = False + delete_spans: list[tuple[int, int]] = [] + for index, (content, span_start, span_end) in enumerate(header_line_spans): + if index == 0: + continue # the status line - while True: - chunk = await self._stream.read(max_bytes, timeout) - if not chunk: - return chunk + if content[:1] in (b" ", b"\t"): + continue # obsolete-line-fold continuation of the previous line - self._buffer += chunk - header_block, separator, rest = self._buffer.partition(b"\r\n\r\n") - if separator: - self._buffer = None - return _merge_duplicate_chunked_transfer_encoding(header_block) + separator + rest + next_content = header_line_spans[index + 1][0] if index + 1 < len(header_line_spans) else b"" + if next_content[:1] in (b" ", b"\t"): + continue # this line has its own fold continuation; leave it alone - if len(self._buffer) > self._max_buffer_size: - buffered = self._buffer - self._buffer = None - return buffered + name, sep, value = content.partition(b":") + if not (sep and name.lower() == b"transfer-encoding" and value.strip(b" \t").lower() == b"chunked"): + continue - async def write(self, buffer: bytes, timeout: float | None = None) -> None: - await self._stream.write(buffer, timeout) + if seen_chunked_transfer_encoding: + delete_spans.append((span_start, span_end)) + else: + seen_chunked_transfer_encoding = True - async def aclose(self) -> None: - await self._stream.aclose() + if not delete_spans: + return header_block - async def start_tls( - self, - ssl_context: ssl.SSLContext, - server_hostname: str | None = None, - timeout: float | None = None, - ) -> AsyncNetworkStream: - return await self._stream.start_tls(ssl_context, server_hostname, timeout) - - def get_extra_info(self, info: str) -> typing.Any: - return self._stream.get_extra_info(info) + merged = bytearray() + cursor = 0 + for delete_start, delete_end in delete_spans: + merged += header_block[cursor:delete_start] + cursor = delete_end + merged += header_block[cursor:] + return bytes(merged) class AsyncHTTP11Connection(AsyncConnectionInterface): @@ -130,7 +133,7 @@ def __init__( keepalive_expiry: float | None = None, ) -> None: self._origin = origin - self._network_stream = AsyncHTTP11ResponseNormalizingStream(stream, self.MAX_INCOMPLETE_EVENT_SIZE) + self._network_stream = stream self._keepalive_expiry: float | None = keepalive_expiry self._expire_at: float | None = None self._state = HTTPConnectionState.NEW @@ -140,6 +143,18 @@ def __init__( our_role=h11.CLIENT, max_incomplete_event_size=self.MAX_INCOMPLETE_EVENT_SIZE, ) + # Accumulates bytes for the response header block currently being + # assembled, so they can be normalized (see + # `_merge_duplicate_chunked_transfer_encoding`) before h11 sees them. + # Only ever appended to while `self._h11_state.their_state` is + # `h11.SEND_RESPONSE` -- see `_receive_event`. + self._response_header_buffer = b"" + # Bytes already read from the network that logically belong to + # whatever comes *after* the header block just flushed above (e.g. a + # 1xx interim response's own headers, followed immediately by the + # final response's headers in the same read) -- reprocessed through + # the same logic before another real network read is attempted. + self._pending_read_ahead = b"" async def handle_async_request(self, request: Request) -> Response: if not self.can_handle_request(request.url.origin): @@ -183,7 +198,7 @@ async def handle_async_request(self, request: Request) -> Response: headers, ) - network_stream: AsyncNetworkStream = self._network_stream + network_stream = self._network_stream # CONNECT or Upgrade request if (status == 101) or ((request.method == b"CONNECT") and (200 <= status < 300)): @@ -257,7 +272,16 @@ async def _receive_response_headers( # raw header casing, rather than the enforced lowercase headers. headers = event.headers.raw_items() - trailing_data, _ = self._h11_state.trailing_data + # `_pending_read_ahead` may hold bytes read alongside this response's + # headers that h11 was never given (see `_receive_event`) -- e.g. the + # leading bytes of an upgraded protocol, read in the same chunk as + # the 101 response's own headers. Combine it with h11's own + # (separately-tracked) trailing data, the same non-destructive read + # in both cases: for an ordinary response, this value is unused by + # the caller and `_pending_read_ahead` is left intact for + # `_receive_response_body`'s own `_receive_event` calls to drain. + h11_trailing_data, _ = self._h11_state.trailing_data + trailing_data = h11_trailing_data + self._pending_read_ahead return http_version, event.status_code, event.reason, headers, trailing_data @@ -278,21 +302,71 @@ async def _receive_event(self, timeout: float | None = None) -> h11.Event | type event = self._h11_state.next_event() if event is h11.NEED_DATA: - data = await self._network_stream.read(self.READ_NUM_BYTES, timeout=timeout) - - # If we feed this case through h11 we'll raise an exception like: - # - # httpcore2.RemoteProtocolError: can't handle event type - # ConnectionClosed when role=SERVER and state=SEND_RESPONSE - # - # Which is accurate, but not very informative from an end-user - # perspective. Instead we handle this case distinctly and treat - # it as a ConnectError. - if data == b"" and self._h11_state.their_state == h11.SEND_RESPONSE: - msg = "Server disconnected without sending a response." - raise RemoteProtocolError(msg) - - self._h11_state.receive_data(data) + if self._pending_read_ahead: + # Bytes already read that belong to whatever comes next + # (see `_pending_read_ahead`'s docstring in `__init__`) -- + # reprocess those before touching the network again. + data, self._pending_read_ahead = self._pending_read_ahead, b"" + else: + data = await self._network_stream.read(self.READ_NUM_BYTES, timeout=timeout) + + # If we feed this case through h11 we'll raise an exception + # like: + # + # httpcore2.RemoteProtocolError: can't handle event type + # ConnectionClosed when role=SERVER and state=SEND_RESPONSE + # + # Which is accurate, but not very informative from an + # end-user perspective. Instead we handle this case + # distinctly and treat it as a ConnectError. + if data == b"" and self._h11_state.their_state == h11.SEND_RESPONSE: + msg = "Server disconnected without sending a response." + raise RemoteProtocolError(msg) + + if self._h11_state.their_state != h11.SEND_RESPONSE or ( + not self._response_header_buffer and self._h11_state.trailing_data[0] + ): + # Either not currently receiving a response's + # status-line/headers (e.g. mid-body) -- nothing to + # normalize, feed it straight through as before. + # + # Or: we're about to *start* accumulating a new header + # block, but h11 is already sitting on unparsed bytes of + # its own (e.g. a pipelined response, or the tail end of + # a previous cycle that arrived in the same read as this + # one). Our boundary search only looks inside our own + # buffer, so if the real header/body boundary straddles + # that hidden junction, searching this new data alone + # could lock onto a later, coincidental match -- inside + # the response body -- and corrupt it. Bail out of + # normalizing this response rather than risk that; h11 + # still handles the duplicate-header case exactly as it + # did before this fix existed. + self._h11_state.receive_data(data) + else: + self._response_header_buffer += data + match = _HEADER_BLOCK_TERMINATOR_RE.search(self._response_header_buffer) + if match is not None: + header_block = self._response_header_buffer[: match.end()] + # Whatever follows is held back rather than fed to h11 + # here -- it may be another header block (an interim + # response ahead of the final one) that still needs + # its own normalization pass, which the top of this + # loop will give it once h11 asks for more data. + self._pending_read_ahead = self._response_header_buffer[match.end() :] + self._response_header_buffer = b"" + self._h11_state.receive_data(_merge_duplicate_chunked_transfer_encoding(header_block)) + elif len(self._response_header_buffer) > self.MAX_INCOMPLETE_EVENT_SIZE: + # No boundary within the size bound h11 itself enforces + # -- stop buffering and let h11 apply its own limit. + buffered = self._response_header_buffer + self._response_header_buffer = b"" + self._h11_state.receive_data(buffered) + # else: boundary not found yet -- loop back without + # feeding h11 anything (and without touching + # `_pending_read_ahead`, which stays empty); `next_event()` + # will return NEED_DATA again, and since there's still no + # read-ahead to drain, this reads the network for more. else: # mypy fails to narrow the type in the above if statement above return event # type: ignore[return-value] @@ -302,7 +376,6 @@ async def _response_closed(self) -> None: if self._h11_state.our_state is h11.DONE and self._h11_state.their_state is h11.DONE: self._state = HTTPConnectionState.IDLE self._h11_state.start_next_cycle() - self._network_stream.reset() if self._keepalive_expiry is not None: now = time.monotonic() self._expire_at = now + self._keepalive_expiry diff --git a/src/httpcore2/httpcore2/_sync/http11.py b/src/httpcore2/httpcore2/_sync/http11.py index 43561374..d087842a 100644 --- a/src/httpcore2/httpcore2/_sync/http11.py +++ b/src/httpcore2/httpcore2/_sync/http11.py @@ -2,6 +2,7 @@ import enum import logging +import re import ssl import time import types @@ -38,85 +39,87 @@ class HTTPConnectionState(enum.IntEnum): CLOSED = 3 -def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: - """ - Merge exact-duplicate `Transfer-Encoding: chunked` header lines into one. - - Some servers send this redundant duplicate on the wire (see - https://github.com/pydantic/httpx2/issues/622). h11 already tolerates - duplicate `Content-Length` headers so long as every value is identical; - this mirrors that same narrow tolerance for `Transfer-Encoding`. Any - other case (differing values, non-`chunked` duplicates) is left - untouched, so h11 still raises for it exactly as before. - """ - status_line, *header_lines = header_block.split(b"\r\n") - - seen_chunked_transfer_encoding = False - merged_lines = [] - for line in header_lines: - name, sep, value = line.partition(b":") - if sep and name.strip().lower() == b"transfer-encoding" and value.strip().lower() == b"chunked": - if seen_chunked_transfer_encoding: - continue - seen_chunked_transfer_encoding = True - merged_lines.append(line) +# Mirrors h11's own header/body boundary (`h11._receivebuffer.blank_line_regex`): +# h11 tolerates a bare `\n` or `\n\r\n`, not just `\r\n\r\n`. +_HEADER_BLOCK_TERMINATOR_RE = re.compile(rb"\n\r?\n") - return b"\r\n".join([status_line, *merged_lines]) - -class HTTP11ResponseNormalizingStream(NetworkStream): +def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: """ - Wraps the underlying network stream and, while a response's headers are - still being received, normalizes them via `_merge_duplicate_chunked_transfer_encoding` - before h11 ever sees the bytes. Once the header block is found (or - `max_buffer_size` is exceeded without finding it), reads pass straight - through unmodified for the rest of that response cycle. + Merge an exact-duplicate `Transfer-Encoding: chunked` header line into an + earlier one, mirroring h11's existing tolerance for duplicate identical + Content-Length headers (see https://github.com/pydantic/httpx2/issues/622). + + `header_block` must end with the header/body boundary matched by + `_HEADER_BLOCK_TERMINATOR_RE`, boundary bytes included. Lines are split + the same way h11 splits them (on `\n`, with one optional trailing `\r` + stripped per line -- see `h11._receivebuffer.ReceiveBuffer.maybe_extract_lines`) + so a header block using non-`\r\n` line endings is parsed identically to + how h11 will parse it. + + Only ever *removes* bytes that are provably an exact, unfolded repeat of + an earlier `Transfer-Encoding: chunked` line: + + - Header names are matched case-insensitively (legitimate per RFC 9110), + but never stripped of surrounding whitespace -- a real header field has + no whitespace between the name and the colon, so anything like + `Transfer-Encoding : chunked` fails to match and is left for h11 to + reject as an illegal header line. + - A candidate line is skipped entirely if it starts with a fold-indicating + space/tab (RFC 9112 obsolete line folding: it's a continuation of the + *previous* header's value, not a standalone header) or if the following + line does -- in the latter case deleting it would orphan that + continuation, changing which header it folds into. + + Any other case (differing values, folded lines, malformed lines) is left + completely untouched, so h11 still raises for it exactly as before. """ + line_spans: list[tuple[bytes, int, int]] = [] + start = 0 + for match in re.finditer(rb"\n", header_block): + end = match.end() + content_end = match.start() + if header_block[content_end - 1 : content_end] == b"\r": + content_end -= 1 + line_spans.append((header_block[start:content_end], start, end)) + start = end + + # The final span is always the second half of the header/body boundary + # itself (mirroring h11's own `del lines[-2:]`), never a real header line. + header_line_spans = line_spans[:-1] - def __init__(self, stream: NetworkStream, max_buffer_size: int) -> None: - self._stream = stream - self._max_buffer_size = max_buffer_size - self._buffer: bytes | None = b"" - - def reset(self) -> None: - self._buffer = b"" - - def read(self, max_bytes: int, timeout: float | None = None) -> bytes: - if self._buffer is None: - return self._stream.read(max_bytes, timeout) + seen_chunked_transfer_encoding = False + delete_spans: list[tuple[int, int]] = [] + for index, (content, span_start, span_end) in enumerate(header_line_spans): + if index == 0: + continue # the status line - while True: - chunk = self._stream.read(max_bytes, timeout) - if not chunk: - return chunk + if content[:1] in (b" ", b"\t"): + continue # obsolete-line-fold continuation of the previous line - self._buffer += chunk - header_block, separator, rest = self._buffer.partition(b"\r\n\r\n") - if separator: - self._buffer = None - return _merge_duplicate_chunked_transfer_encoding(header_block) + separator + rest + next_content = header_line_spans[index + 1][0] if index + 1 < len(header_line_spans) else b"" + if next_content[:1] in (b" ", b"\t"): + continue # this line has its own fold continuation; leave it alone - if len(self._buffer) > self._max_buffer_size: - buffered = self._buffer - self._buffer = None - return buffered + name, sep, value = content.partition(b":") + if not (sep and name.lower() == b"transfer-encoding" and value.strip(b" \t").lower() == b"chunked"): + continue - def write(self, buffer: bytes, timeout: float | None = None) -> None: - self._stream.write(buffer, timeout) + if seen_chunked_transfer_encoding: + delete_spans.append((span_start, span_end)) + else: + seen_chunked_transfer_encoding = True - def close(self) -> None: - self._stream.close() + if not delete_spans: + return header_block - def start_tls( - self, - ssl_context: ssl.SSLContext, - server_hostname: str | None = None, - timeout: float | None = None, - ) -> NetworkStream: - return self._stream.start_tls(ssl_context, server_hostname, timeout) - - def get_extra_info(self, info: str) -> typing.Any: - return self._stream.get_extra_info(info) + merged = bytearray() + cursor = 0 + for delete_start, delete_end in delete_spans: + merged += header_block[cursor:delete_start] + cursor = delete_end + merged += header_block[cursor:] + return bytes(merged) class HTTP11Connection(ConnectionInterface): @@ -130,7 +133,7 @@ def __init__( keepalive_expiry: float | None = None, ) -> None: self._origin = origin - self._network_stream = HTTP11ResponseNormalizingStream(stream, self.MAX_INCOMPLETE_EVENT_SIZE) + self._network_stream = stream self._keepalive_expiry: float | None = keepalive_expiry self._expire_at: float | None = None self._state = HTTPConnectionState.NEW @@ -140,6 +143,18 @@ def __init__( our_role=h11.CLIENT, max_incomplete_event_size=self.MAX_INCOMPLETE_EVENT_SIZE, ) + # Accumulates bytes for the response header block currently being + # assembled, so they can be normalized (see + # `_merge_duplicate_chunked_transfer_encoding`) before h11 sees them. + # Only ever appended to while `self._h11_state.their_state` is + # `h11.SEND_RESPONSE` -- see `_receive_event`. + self._response_header_buffer = b"" + # Bytes already read from the network that logically belong to + # whatever comes *after* the header block just flushed above (e.g. a + # 1xx interim response's own headers, followed immediately by the + # final response's headers in the same read) -- reprocessed through + # the same logic before another real network read is attempted. + self._pending_read_ahead = b"" def handle_request(self, request: Request) -> Response: if not self.can_handle_request(request.url.origin): @@ -183,7 +198,7 @@ def handle_request(self, request: Request) -> Response: headers, ) - network_stream: NetworkStream = self._network_stream + network_stream = self._network_stream # CONNECT or Upgrade request if (status == 101) or ((request.method == b"CONNECT") and (200 <= status < 300)): @@ -257,7 +272,16 @@ def _receive_response_headers( # raw header casing, rather than the enforced lowercase headers. headers = event.headers.raw_items() - trailing_data, _ = self._h11_state.trailing_data + # `_pending_read_ahead` may hold bytes read alongside this response's + # headers that h11 was never given (see `_receive_event`) -- e.g. the + # leading bytes of an upgraded protocol, read in the same chunk as + # the 101 response's own headers. Combine it with h11's own + # (separately-tracked) trailing data, the same non-destructive read + # in both cases: for an ordinary response, this value is unused by + # the caller and `_pending_read_ahead` is left intact for + # `_receive_response_body`'s own `_receive_event` calls to drain. + h11_trailing_data, _ = self._h11_state.trailing_data + trailing_data = h11_trailing_data + self._pending_read_ahead return http_version, event.status_code, event.reason, headers, trailing_data @@ -278,21 +302,71 @@ def _receive_event(self, timeout: float | None = None) -> h11.Event | type[h11.P event = self._h11_state.next_event() if event is h11.NEED_DATA: - data = self._network_stream.read(self.READ_NUM_BYTES, timeout=timeout) - - # If we feed this case through h11 we'll raise an exception like: - # - # httpcore2.RemoteProtocolError: can't handle event type - # ConnectionClosed when role=SERVER and state=SEND_RESPONSE - # - # Which is accurate, but not very informative from an end-user - # perspective. Instead we handle this case distinctly and treat - # it as a ConnectError. - if data == b"" and self._h11_state.their_state == h11.SEND_RESPONSE: - msg = "Server disconnected without sending a response." - raise RemoteProtocolError(msg) - - self._h11_state.receive_data(data) + if self._pending_read_ahead: + # Bytes already read that belong to whatever comes next + # (see `_pending_read_ahead`'s docstring in `__init__`) -- + # reprocess those before touching the network again. + data, self._pending_read_ahead = self._pending_read_ahead, b"" + else: + data = self._network_stream.read(self.READ_NUM_BYTES, timeout=timeout) + + # If we feed this case through h11 we'll raise an exception + # like: + # + # httpcore2.RemoteProtocolError: can't handle event type + # ConnectionClosed when role=SERVER and state=SEND_RESPONSE + # + # Which is accurate, but not very informative from an + # end-user perspective. Instead we handle this case + # distinctly and treat it as a ConnectError. + if data == b"" and self._h11_state.their_state == h11.SEND_RESPONSE: + msg = "Server disconnected without sending a response." + raise RemoteProtocolError(msg) + + if self._h11_state.their_state != h11.SEND_RESPONSE or ( + not self._response_header_buffer and self._h11_state.trailing_data[0] + ): + # Either not currently receiving a response's + # status-line/headers (e.g. mid-body) -- nothing to + # normalize, feed it straight through as before. + # + # Or: we're about to *start* accumulating a new header + # block, but h11 is already sitting on unparsed bytes of + # its own (e.g. a pipelined response, or the tail end of + # a previous cycle that arrived in the same read as this + # one). Our boundary search only looks inside our own + # buffer, so if the real header/body boundary straddles + # that hidden junction, searching this new data alone + # could lock onto a later, coincidental match -- inside + # the response body -- and corrupt it. Bail out of + # normalizing this response rather than risk that; h11 + # still handles the duplicate-header case exactly as it + # did before this fix existed. + self._h11_state.receive_data(data) + else: + self._response_header_buffer += data + match = _HEADER_BLOCK_TERMINATOR_RE.search(self._response_header_buffer) + if match is not None: + header_block = self._response_header_buffer[: match.end()] + # Whatever follows is held back rather than fed to h11 + # here -- it may be another header block (an interim + # response ahead of the final one) that still needs + # its own normalization pass, which the top of this + # loop will give it once h11 asks for more data. + self._pending_read_ahead = self._response_header_buffer[match.end() :] + self._response_header_buffer = b"" + self._h11_state.receive_data(_merge_duplicate_chunked_transfer_encoding(header_block)) + elif len(self._response_header_buffer) > self.MAX_INCOMPLETE_EVENT_SIZE: + # No boundary within the size bound h11 itself enforces + # -- stop buffering and let h11 apply its own limit. + buffered = self._response_header_buffer + self._response_header_buffer = b"" + self._h11_state.receive_data(buffered) + # else: boundary not found yet -- loop back without + # feeding h11 anything (and without touching + # `_pending_read_ahead`, which stays empty); `next_event()` + # will return NEED_DATA again, and since there's still no + # read-ahead to drain, this reads the network for more. else: # mypy fails to narrow the type in the above if statement above return event # type: ignore[return-value] @@ -302,7 +376,6 @@ def _response_closed(self) -> None: if self._h11_state.our_state is h11.DONE and self._h11_state.their_state is h11.DONE: self._state = HTTPConnectionState.IDLE self._h11_state.start_next_cycle() - self._network_stream.reset() if self._keepalive_expiry is not None: now = time.monotonic() self._expire_at = now + self._keepalive_expiry diff --git a/tests/httpcore2/_async/test_http11.py b/tests/httpcore2/_async/test_http11.py index 8c3e6736..fb1a54f7 100644 --- a/tests/httpcore2/_async/test_http11.py +++ b/tests/httpcore2/_async/test_http11.py @@ -422,6 +422,148 @@ async def test_http11_connection_with_oversized_headers_and_no_terminator() -> N await conn.request("GET", "https://example.com/") +@pytest.mark.anyio +async def test_http11_connection_merges_duplicate_transfer_encoding_with_lf_terminated_headers() -> None: + """ + h11 tolerates bare `\\n` (not just `\\r\\n`) as a header line ending, so + the merge must recognize the header/body boundary and split lines the + same way h11 does, not assume `\\r\\n` throughout. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.AsyncMockStream( + [ + b"HTTP/1.1 200 OK\n", + b"Content-Type: text/plain\n", + b"Transfer-Encoding: chunked\n", + b"Transfer-Encoding: chunked\n", + b"\n", + b"5\r\nHello\r\n0\r\n\r\n", + ] + ) + async with httpcore2.AsyncHTTP11Connection(origin=origin, stream=stream) as conn: + response = await conn.request("GET", "https://example.com/") + assert response.status == 200 + assert response.content == b"Hello" + + transfer_encodings = [v for k, v in response.headers if k.lower() == b"transfer-encoding"] + assert transfer_encodings == [b"chunked"] + + +@pytest.mark.anyio +async def test_http11_connection_merges_duplicate_transfer_encoding_after_interim_response() -> None: + """ + A `100 Continue` (or other 1xx) response ahead of the final response must + not disable normalization for the final response's own headers. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.AsyncMockStream( + [ + b"HTTP/1.1 100 Continue\r\n", + b"\r\n", + b"HTTP/1.1 200 OK\r\n", + b"Transfer-Encoding: chunked\r\n", + b"Transfer-Encoding: chunked\r\n", + b"\r\n", + b"5\r\nHello\r\n0\r\n\r\n", + ] + ) + async with httpcore2.AsyncHTTP11Connection(origin=origin, stream=stream) as conn: + response = await conn.request( + "GET", + "https://example.com/", + headers={"Expect": "continue"}, + ) + assert response.status == 200 + assert response.content == b"Hello" + + transfer_encodings = [v for k, v in response.headers if k.lower() == b"transfer-encoding"] + assert transfer_encodings == [b"chunked"] + + +@pytest.mark.anyio +async def test_http11_connection_merges_duplicate_transfer_encoding_after_interim_response_same_read() -> None: + """ + Same as above, but the interim response and the final response's headers + arrive in a single network read together -- h11 doesn't need another + `NEED_DATA` round trip to see the final response's headers, so they must + still get normalized even though no further data is read from the + network in between. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.AsyncMockStream( + [ + b"HTTP/1.1 100 Continue\r\n\r\n" + b"HTTP/1.1 200 OK\r\n" + b"Transfer-Encoding: chunked\r\n" + b"Transfer-Encoding: chunked\r\n" + b"\r\n" + b"5\r\nHello\r\n0\r\n\r\n" + ] + ) + async with httpcore2.AsyncHTTP11Connection(origin=origin, stream=stream) as conn: + response = await conn.request( + "GET", + "https://example.com/", + headers={"Expect": "continue"}, + ) + assert response.status == 200 + assert response.content == b"Hello" + + transfer_encodings = [v for k, v in response.headers if k.lower() == b"transfer-encoding"] + assert transfer_encodings == [b"chunked"] + + +@pytest.mark.anyio +async def test_http11_connection_does_not_merge_transfer_encoding_with_space_before_colon() -> None: + """ + `Transfer-Encoding : chunked` (space before the colon) is not the same + raw header line as `Transfer-Encoding: chunked` -- it's illegal per the + header-field grammar. It must not be treated as an equivalent duplicate; + h11 should still see it and reject the message. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.AsyncMockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"Transfer-Encoding: chunked\r\n", + b"Transfer-Encoding : chunked\r\n", + b"\r\n", + b"5\r\nHello\r\n0\r\n\r\n", + ] + ) + async with httpcore2.AsyncHTTP11Connection(origin=origin, stream=stream) as conn: + with pytest.raises(httpcore2.RemoteProtocolError): + await conn.request("GET", "https://example.com/") + + +@pytest.mark.anyio +async def test_http11_connection_does_not_merge_obsolete_line_folded_transfer_encoding() -> None: + """ + Obsolete line folding (RFC 7230 3.2.4) means a header line starting with + whitespace is a *continuation* of the previous header's value, not a + standalone header. A folded line that happens to read + `Transfer-Encoding: chunked` must never be treated as a duplicate to + merge away -- doing so would delete part of an unrelated header's value + and let an otherwise-invalid message through. h11 must still see the + fold and reject the message exactly as it would unpatched. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.AsyncMockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"X-Cache: HIT\r\n", + b" Transfer-Encoding: chunked\r\n", + b"Content-Length: 5\r\n", + b" Transfer-Encoding: chunked\r\n", + b"\r\n", + b"Hello", + ] + ) + async with httpcore2.AsyncHTTP11Connection(origin=origin, stream=stream) as conn: + with pytest.raises(httpcore2.RemoteProtocolError): + await conn.request("GET", "https://example.com/") + + @pytest.mark.anyio async def test_http11_header_sub_100kb() -> None: """ diff --git a/tests/httpcore2/_sync/test_http11.py b/tests/httpcore2/_sync/test_http11.py index 9e2cde67..43778d22 100644 --- a/tests/httpcore2/_sync/test_http11.py +++ b/tests/httpcore2/_sync/test_http11.py @@ -423,6 +423,148 @@ def test_http11_connection_with_oversized_headers_and_no_terminator() -> None: +def test_http11_connection_merges_duplicate_transfer_encoding_with_lf_terminated_headers() -> None: + """ + h11 tolerates bare `\\n` (not just `\\r\\n`) as a header line ending, so + the merge must recognize the header/body boundary and split lines the + same way h11 does, not assume `\\r\\n` throughout. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.MockStream( + [ + b"HTTP/1.1 200 OK\n", + b"Content-Type: text/plain\n", + b"Transfer-Encoding: chunked\n", + b"Transfer-Encoding: chunked\n", + b"\n", + b"5\r\nHello\r\n0\r\n\r\n", + ] + ) + with httpcore2.HTTP11Connection(origin=origin, stream=stream) as conn: + response = conn.request("GET", "https://example.com/") + assert response.status == 200 + assert response.content == b"Hello" + + transfer_encodings = [v for k, v in response.headers if k.lower() == b"transfer-encoding"] + assert transfer_encodings == [b"chunked"] + + + +def test_http11_connection_merges_duplicate_transfer_encoding_after_interim_response() -> None: + """ + A `100 Continue` (or other 1xx) response ahead of the final response must + not disable normalization for the final response's own headers. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.MockStream( + [ + b"HTTP/1.1 100 Continue\r\n", + b"\r\n", + b"HTTP/1.1 200 OK\r\n", + b"Transfer-Encoding: chunked\r\n", + b"Transfer-Encoding: chunked\r\n", + b"\r\n", + b"5\r\nHello\r\n0\r\n\r\n", + ] + ) + with httpcore2.HTTP11Connection(origin=origin, stream=stream) as conn: + response = conn.request( + "GET", + "https://example.com/", + headers={"Expect": "continue"}, + ) + assert response.status == 200 + assert response.content == b"Hello" + + transfer_encodings = [v for k, v in response.headers if k.lower() == b"transfer-encoding"] + assert transfer_encodings == [b"chunked"] + + + +def test_http11_connection_merges_duplicate_transfer_encoding_after_interim_response_same_read() -> None: + """ + Same as above, but the interim response and the final response's headers + arrive in a single network read together -- h11 doesn't need another + `NEED_DATA` round trip to see the final response's headers, so they must + still get normalized even though no further data is read from the + network in between. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.MockStream( + [ + b"HTTP/1.1 100 Continue\r\n\r\n" + b"HTTP/1.1 200 OK\r\n" + b"Transfer-Encoding: chunked\r\n" + b"Transfer-Encoding: chunked\r\n" + b"\r\n" + b"5\r\nHello\r\n0\r\n\r\n" + ] + ) + with httpcore2.HTTP11Connection(origin=origin, stream=stream) as conn: + response = conn.request( + "GET", + "https://example.com/", + headers={"Expect": "continue"}, + ) + assert response.status == 200 + assert response.content == b"Hello" + + transfer_encodings = [v for k, v in response.headers if k.lower() == b"transfer-encoding"] + assert transfer_encodings == [b"chunked"] + + + +def test_http11_connection_does_not_merge_transfer_encoding_with_space_before_colon() -> None: + """ + `Transfer-Encoding : chunked` (space before the colon) is not the same + raw header line as `Transfer-Encoding: chunked` -- it's illegal per the + header-field grammar. It must not be treated as an equivalent duplicate; + h11 should still see it and reject the message. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.MockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"Transfer-Encoding: chunked\r\n", + b"Transfer-Encoding : chunked\r\n", + b"\r\n", + b"5\r\nHello\r\n0\r\n\r\n", + ] + ) + with httpcore2.HTTP11Connection(origin=origin, stream=stream) as conn: + with pytest.raises(httpcore2.RemoteProtocolError): + conn.request("GET", "https://example.com/") + + + +def test_http11_connection_does_not_merge_obsolete_line_folded_transfer_encoding() -> None: + """ + Obsolete line folding (RFC 7230 3.2.4) means a header line starting with + whitespace is a *continuation* of the previous header's value, not a + standalone header. A folded line that happens to read + `Transfer-Encoding: chunked` must never be treated as a duplicate to + merge away -- doing so would delete part of an unrelated header's value + and let an otherwise-invalid message through. h11 must still see the + fold and reject the message exactly as it would unpatched. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.MockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"X-Cache: HIT\r\n", + b" Transfer-Encoding: chunked\r\n", + b"Content-Length: 5\r\n", + b" Transfer-Encoding: chunked\r\n", + b"\r\n", + b"Hello", + ] + ) + with httpcore2.HTTP11Connection(origin=origin, stream=stream) as conn: + with pytest.raises(httpcore2.RemoteProtocolError): + conn.request("GET", "https://example.com/") + + + def test_http11_header_sub_100kb() -> None: """ A connection should be able to handle a http header size up to 100kB. From 9d3d5aa8b967ad3b97a46a106b4fe76b2c9f3d0f Mon Sep 17 00:00:00 2001 From: Andres Rivero Date: Fri, 11 Sep 2026 19:06:26 -0700 Subject: [PATCH 3/6] fix: avoid quadratic header scan and CL/TE ambiguity in TE merge A third security review of the previous commit's fix found two more issues, neither a correctness/desync bug like the earlier two rounds: - The response-header accumulator re-searched for the header/body boundary from byte 0 on every network read instead of resuming where the previous search left off, making header assembly O(n^2) in the header size. h11's own ReceiveBuffer avoids exactly this (its module docstring calls it out explicitly as a DoS concern) via a resumable search offset; the accumulator now does the same, tracking `_response_header_search_from` and resuming 2 bytes before the end of what's already been scanned (the terminator is at most 3 bytes). Isolated benchmarking confirms linear scaling after the fix, versus quadratic before (~65x slower at 100KB). - Merging duplicate `Transfer-Encoding: chunked` when a `Content-Length` header is also present let a Content-Length/Transfer-Encoding framing ambiguity through that h11 previously rejected outright -- exactly the shape of the classic conflicting-framing request-smuggling primitive. Issue #622's actual reproductions never combine the two, so the merge now bails out (deliberately broad, case-insensitive substring check) whenever anything resembling Content-Length is present in the header block, at no cost to the fix's actual purpose. Reproduced both issues before fixing (isolated O(n) vs O(n^2) timing comparison; a Content-Length + duplicate-Transfer-Encoding payload that previously merged and desynced a pooled connection), and re-verified every proof-of-concept from the first two review rounds stays fixed. Full suite (2026 tests) passes with 100% coverage; mypy strict and ruff are clean. Fixes #622 --- src/httpcore2/httpcore2/_async/http11.py | 58 ++++++++++++++++++------ src/httpcore2/httpcore2/_sync/http11.py | 58 ++++++++++++++++++------ tests/httpcore2/_async/test_http11.py | 49 ++++++++++++++++++++ tests/httpcore2/_sync/test_http11.py | 49 ++++++++++++++++++++ 4 files changed, 188 insertions(+), 26 deletions(-) diff --git a/src/httpcore2/httpcore2/_async/http11.py b/src/httpcore2/httpcore2/_async/http11.py index 39cf69c4..3d0fd4b5 100644 --- a/src/httpcore2/httpcore2/_async/http11.py +++ b/src/httpcore2/httpcore2/_async/http11.py @@ -73,7 +73,18 @@ def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: Any other case (differing values, folded lines, malformed lines) is left completely untouched, so h11 still raises for it exactly as before. + + Never applies if the header block also contains anything resembling a + `Content-Length` header (a deliberately broad, case-insensitive substring + check, not a precise parse). `Transfer-Encoding` combined with + `Content-Length` is exactly the shape of the classic conflicting-framing + request-smuggling primitive that RFC 9112 requires treating as an error; + issue #622's actual reproductions never combine the two, so giving up the + merge here costs nothing while closing off that class of ambiguity. """ + if b"content-length" in header_block.lower(): + return header_block + line_spans: list[tuple[bytes, int, int]] = [] start = 0 for match in re.finditer(rb"\n", header_block): @@ -147,8 +158,19 @@ def __init__( # assembled, so they can be normalized (see # `_merge_duplicate_chunked_transfer_encoding`) before h11 sees them. # Only ever appended to while `self._h11_state.their_state` is - # `h11.SEND_RESPONSE` -- see `_receive_event`. - self._response_header_buffer = b"" + # `h11.SEND_RESPONSE` -- see `_receive_event`. A `bytearray` (not + # `bytes`) so repeated `+=` don't reallocate-and-copy the whole thing + # each time. + self._response_header_buffer = bytearray() + # How far into `_response_header_buffer` the terminator search has + # already ruled out a match, so each new read only rescans the tail + # instead of the whole accumulated buffer -- mirrors h11's own + # `ReceiveBuffer._multiple_lines_search` (see its module docstring: + # "reading short segments out of a long buffer MUST be O(bytes read) + # to avoid DoS issues"). The terminator is at most 3 bytes, so it's + # always safe to resume 2 bytes before the end of what's already + # been scanned. + self._response_header_search_from = 0 # Bytes already read from the network that logically belong to # whatever comes *after* the header block just flushed above (e.g. a # 1xx interim response's own headers, followed immediately by the @@ -345,28 +367,38 @@ async def _receive_event(self, timeout: float | None = None) -> h11.Event | type self._h11_state.receive_data(data) else: self._response_header_buffer += data - match = _HEADER_BLOCK_TERMINATOR_RE.search(self._response_header_buffer) + match = _HEADER_BLOCK_TERMINATOR_RE.search( + self._response_header_buffer, self._response_header_search_from + ) if match is not None: - header_block = self._response_header_buffer[: match.end()] + header_block = bytes(self._response_header_buffer[: match.end()]) # Whatever follows is held back rather than fed to h11 # here -- it may be another header block (an interim # response ahead of the final one) that still needs # its own normalization pass, which the top of this # loop will give it once h11 asks for more data. - self._pending_read_ahead = self._response_header_buffer[match.end() :] - self._response_header_buffer = b"" + self._pending_read_ahead = bytes(self._response_header_buffer[match.end() :]) + self._response_header_buffer = bytearray() + self._response_header_search_from = 0 self._h11_state.receive_data(_merge_duplicate_chunked_transfer_encoding(header_block)) elif len(self._response_header_buffer) > self.MAX_INCOMPLETE_EVENT_SIZE: # No boundary within the size bound h11 itself enforces # -- stop buffering and let h11 apply its own limit. - buffered = self._response_header_buffer - self._response_header_buffer = b"" + buffered = bytes(self._response_header_buffer) + self._response_header_buffer = bytearray() + self._response_header_search_from = 0 self._h11_state.receive_data(buffered) - # else: boundary not found yet -- loop back without - # feeding h11 anything (and without touching - # `_pending_read_ahead`, which stays empty); `next_event()` - # will return NEED_DATA again, and since there's still no - # read-ahead to drain, this reads the network for more. + else: + # Boundary not found yet -- loop back without feeding + # h11 anything (and without touching + # `_pending_read_ahead`, which stays empty); next_event() + # will return NEED_DATA again, and since there's still + # no read-ahead to drain, this reads the network for + # more. The terminator is at most 3 bytes, so the next + # search can safely skip everything except the last 2 + # bytes already scanned -- without this, accumulating + # a large header block byte-by-byte is O(n^2). + self._response_header_search_from = max(0, len(self._response_header_buffer) - 2) else: # mypy fails to narrow the type in the above if statement above return event # type: ignore[return-value] diff --git a/src/httpcore2/httpcore2/_sync/http11.py b/src/httpcore2/httpcore2/_sync/http11.py index d087842a..f186eb83 100644 --- a/src/httpcore2/httpcore2/_sync/http11.py +++ b/src/httpcore2/httpcore2/_sync/http11.py @@ -73,7 +73,18 @@ def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: Any other case (differing values, folded lines, malformed lines) is left completely untouched, so h11 still raises for it exactly as before. + + Never applies if the header block also contains anything resembling a + `Content-Length` header (a deliberately broad, case-insensitive substring + check, not a precise parse). `Transfer-Encoding` combined with + `Content-Length` is exactly the shape of the classic conflicting-framing + request-smuggling primitive that RFC 9112 requires treating as an error; + issue #622's actual reproductions never combine the two, so giving up the + merge here costs nothing while closing off that class of ambiguity. """ + if b"content-length" in header_block.lower(): + return header_block + line_spans: list[tuple[bytes, int, int]] = [] start = 0 for match in re.finditer(rb"\n", header_block): @@ -147,8 +158,19 @@ def __init__( # assembled, so they can be normalized (see # `_merge_duplicate_chunked_transfer_encoding`) before h11 sees them. # Only ever appended to while `self._h11_state.their_state` is - # `h11.SEND_RESPONSE` -- see `_receive_event`. - self._response_header_buffer = b"" + # `h11.SEND_RESPONSE` -- see `_receive_event`. A `bytearray` (not + # `bytes`) so repeated `+=` don't reallocate-and-copy the whole thing + # each time. + self._response_header_buffer = bytearray() + # How far into `_response_header_buffer` the terminator search has + # already ruled out a match, so each new read only rescans the tail + # instead of the whole accumulated buffer -- mirrors h11's own + # `ReceiveBuffer._multiple_lines_search` (see its module docstring: + # "reading short segments out of a long buffer MUST be O(bytes read) + # to avoid DoS issues"). The terminator is at most 3 bytes, so it's + # always safe to resume 2 bytes before the end of what's already + # been scanned. + self._response_header_search_from = 0 # Bytes already read from the network that logically belong to # whatever comes *after* the header block just flushed above (e.g. a # 1xx interim response's own headers, followed immediately by the @@ -345,28 +367,38 @@ def _receive_event(self, timeout: float | None = None) -> h11.Event | type[h11.P self._h11_state.receive_data(data) else: self._response_header_buffer += data - match = _HEADER_BLOCK_TERMINATOR_RE.search(self._response_header_buffer) + match = _HEADER_BLOCK_TERMINATOR_RE.search( + self._response_header_buffer, self._response_header_search_from + ) if match is not None: - header_block = self._response_header_buffer[: match.end()] + header_block = bytes(self._response_header_buffer[: match.end()]) # Whatever follows is held back rather than fed to h11 # here -- it may be another header block (an interim # response ahead of the final one) that still needs # its own normalization pass, which the top of this # loop will give it once h11 asks for more data. - self._pending_read_ahead = self._response_header_buffer[match.end() :] - self._response_header_buffer = b"" + self._pending_read_ahead = bytes(self._response_header_buffer[match.end() :]) + self._response_header_buffer = bytearray() + self._response_header_search_from = 0 self._h11_state.receive_data(_merge_duplicate_chunked_transfer_encoding(header_block)) elif len(self._response_header_buffer) > self.MAX_INCOMPLETE_EVENT_SIZE: # No boundary within the size bound h11 itself enforces # -- stop buffering and let h11 apply its own limit. - buffered = self._response_header_buffer - self._response_header_buffer = b"" + buffered = bytes(self._response_header_buffer) + self._response_header_buffer = bytearray() + self._response_header_search_from = 0 self._h11_state.receive_data(buffered) - # else: boundary not found yet -- loop back without - # feeding h11 anything (and without touching - # `_pending_read_ahead`, which stays empty); `next_event()` - # will return NEED_DATA again, and since there's still no - # read-ahead to drain, this reads the network for more. + else: + # Boundary not found yet -- loop back without feeding + # h11 anything (and without touching + # `_pending_read_ahead`, which stays empty); next_event() + # will return NEED_DATA again, and since there's still + # no read-ahead to drain, this reads the network for + # more. The terminator is at most 3 bytes, so the next + # search can safely skip everything except the last 2 + # bytes already scanned -- without this, accumulating + # a large header block byte-by-byte is O(n^2). + self._response_header_search_from = max(0, len(self._response_header_buffer) - 2) else: # mypy fails to narrow the type in the above if statement above return event # type: ignore[return-value] diff --git a/tests/httpcore2/_async/test_http11.py b/tests/httpcore2/_async/test_http11.py index fb1a54f7..0b685916 100644 --- a/tests/httpcore2/_async/test_http11.py +++ b/tests/httpcore2/_async/test_http11.py @@ -402,6 +402,31 @@ async def test_http11_connection_with_conflicting_transfer_encoding_headers() -> await conn.request("GET", "https://example.com/") +@pytest.mark.anyio +async def test_http11_connection_does_not_merge_transfer_encoding_alongside_content_length() -> None: + """ + `Transfer-Encoding` combined with `Content-Length` is exactly the shape + of the classic conflicting-framing request-smuggling primitive, so the + merge must never apply when a `Content-Length` header is also present -- + even though the duplicate `Transfer-Encoding` lines are themselves + byte-identical -- leaving h11 to reject the message as before. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.AsyncMockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"Content-Length: 46\r\n", + b"Transfer-Encoding: chunked\r\n", + b"Transfer-Encoding: chunked\r\n", + b"\r\n", + b"5\r\nHello\r\n0\r\n\r\n", + ] + ) + async with httpcore2.AsyncHTTP11Connection(origin=origin, stream=stream) as conn: + with pytest.raises(httpcore2.RemoteProtocolError): + await conn.request("GET", "https://example.com/") + + @pytest.mark.anyio async def test_http11_connection_with_oversized_headers_and_no_terminator() -> None: """ @@ -564,6 +589,30 @@ async def test_http11_connection_does_not_merge_obsolete_line_folded_transfer_en await conn.request("GET", "https://example.com/") +@pytest.mark.anyio +async def test_http11_connection_does_not_merge_obsolete_line_folded_transfer_encoding_without_content_length() -> None: + """ + Same fold-continuation hazard as above, but without a `Content-Length` + header present, so this exercises the fold-continuation skip in + `_merge_duplicate_chunked_transfer_encoding` directly rather than via + the (separate) `Content-Length` bail-out. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.AsyncMockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b" Transfer-Encoding: chunked\r\n", + b"Transfer-Encoding: chunked\r\n", + b" folded-continuation\r\n", + b"\r\n", + b"", + ] + ) + async with httpcore2.AsyncHTTP11Connection(origin=origin, stream=stream) as conn: + with pytest.raises(httpcore2.RemoteProtocolError): + await conn.request("GET", "https://example.com/") + + @pytest.mark.anyio async def test_http11_header_sub_100kb() -> None: """ diff --git a/tests/httpcore2/_sync/test_http11.py b/tests/httpcore2/_sync/test_http11.py index 43778d22..acddef80 100644 --- a/tests/httpcore2/_sync/test_http11.py +++ b/tests/httpcore2/_sync/test_http11.py @@ -403,6 +403,31 @@ def test_http11_connection_with_conflicting_transfer_encoding_headers() -> None: +def test_http11_connection_does_not_merge_transfer_encoding_alongside_content_length() -> None: + """ + `Transfer-Encoding` combined with `Content-Length` is exactly the shape + of the classic conflicting-framing request-smuggling primitive, so the + merge must never apply when a `Content-Length` header is also present -- + even though the duplicate `Transfer-Encoding` lines are themselves + byte-identical -- leaving h11 to reject the message as before. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.MockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b"Content-Length: 46\r\n", + b"Transfer-Encoding: chunked\r\n", + b"Transfer-Encoding: chunked\r\n", + b"\r\n", + b"5\r\nHello\r\n0\r\n\r\n", + ] + ) + with httpcore2.HTTP11Connection(origin=origin, stream=stream) as conn: + with pytest.raises(httpcore2.RemoteProtocolError): + conn.request("GET", "https://example.com/") + + + def test_http11_connection_with_oversized_headers_and_no_terminator() -> None: """ If the header block never terminates and grows past the incomplete-event @@ -565,6 +590,30 @@ def test_http11_connection_does_not_merge_obsolete_line_folded_transfer_encoding +def test_http11_connection_does_not_merge_obsolete_line_folded_transfer_encoding_without_content_length() -> None: + """ + Same fold-continuation hazard as above, but without a `Content-Length` + header present, so this exercises the fold-continuation skip in + `_merge_duplicate_chunked_transfer_encoding` directly rather than via + the (separate) `Content-Length` bail-out. + """ + origin = httpcore2.Origin(b"https", b"example.com", 443) + stream = httpcore2.MockStream( + [ + b"HTTP/1.1 200 OK\r\n", + b" Transfer-Encoding: chunked\r\n", + b"Transfer-Encoding: chunked\r\n", + b" folded-continuation\r\n", + b"\r\n", + b"", + ] + ) + with httpcore2.HTTP11Connection(origin=origin, stream=stream) as conn: + with pytest.raises(httpcore2.RemoteProtocolError): + conn.request("GET", "https://example.com/") + + + def test_http11_header_sub_100kb() -> None: """ A connection should be able to handle a http header size up to 100kB. From dfd2630a00360b0fba153996324017a7b125db70 Mon Sep 17 00:00:00 2001 From: drusc0 Date: Fri, 11 Sep 2026 20:21:02 -0700 Subject: [PATCH 4/6] Update src/httpcore2/httpcore2/_async/http11.py Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- src/httpcore2/httpcore2/_async/http11.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/httpcore2/httpcore2/_async/http11.py b/src/httpcore2/httpcore2/_async/http11.py index 3d0fd4b5..34938d2f 100644 --- a/src/httpcore2/httpcore2/_async/http11.py +++ b/src/httpcore2/httpcore2/_async/http11.py @@ -82,7 +82,10 @@ def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: issue #622's actual reproductions never combine the two, so giving up the merge here costs nothing while closing off that class of ambiguity. """ - if b"content-length" in header_block.lower(): + if any( + line.partition(b":")[0].lower() == b"content-length" + for line in header_block.split(b"\n") + ): return header_block line_spans: list[tuple[bytes, int, int]] = [] From 4e687ff9170ff0e974210ec8a107214f2bcc1080 Mon Sep 17 00:00:00 2001 From: Andres Rivero Date: Fri, 11 Sep 2026 20:25:30 -0700 Subject: [PATCH 5/6] fix: format and regenerate sync mirror for CL check precision fix The reviewer's suggested change (narrowing the Content-Length guard to check each header line's field name before the colon, rather than a substring search over the whole raw block -- avoiding a false positive when an unrelated header value or the reason phrase merely contains the text "content-length") was applied directly to the async source via GitHub's UI, which doesn't run this repo's formatting/unasync pipeline. This runs scripts/lint to reformat per ruff's style and regenerate the auto-generated sync mirror, which is what CI was failing on. Verified the precision fix itself is correct: a response whose reason phrase/header values merely mention "content-length" now merges the duplicate Transfer-Encoding header as it should, while a genuine Content-Length header still blocks the merge. Full suite (2026 tests) passes with 100% coverage; mypy strict and ruff are clean. --- src/httpcore2/httpcore2/_async/http11.py | 5 +---- src/httpcore2/httpcore2/_sync/http11.py | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/httpcore2/httpcore2/_async/http11.py b/src/httpcore2/httpcore2/_async/http11.py index 34938d2f..91fb7eb0 100644 --- a/src/httpcore2/httpcore2/_async/http11.py +++ b/src/httpcore2/httpcore2/_async/http11.py @@ -82,10 +82,7 @@ def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: issue #622's actual reproductions never combine the two, so giving up the merge here costs nothing while closing off that class of ambiguity. """ - if any( - line.partition(b":")[0].lower() == b"content-length" - for line in header_block.split(b"\n") - ): + if any(line.partition(b":")[0].lower() == b"content-length" for line in header_block.split(b"\n")): return header_block line_spans: list[tuple[bytes, int, int]] = [] diff --git a/src/httpcore2/httpcore2/_sync/http11.py b/src/httpcore2/httpcore2/_sync/http11.py index f186eb83..14e29328 100644 --- a/src/httpcore2/httpcore2/_sync/http11.py +++ b/src/httpcore2/httpcore2/_sync/http11.py @@ -82,7 +82,7 @@ def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: issue #622's actual reproductions never combine the two, so giving up the merge here costs nothing while closing off that class of ambiguity. """ - if b"content-length" in header_block.lower(): + if any(line.partition(b":")[0].lower() == b"content-length" for line in header_block.split(b"\n")): return header_block line_spans: list[tuple[bytes, int, int]] = [] From 6d65e31d1701900f7441682777bc4aa1bb6274b2 Mon Sep 17 00:00:00 2001 From: Andres Rivero Date: Fri, 11 Sep 2026 21:17:00 -0700 Subject: [PATCH 6/6] docs: correct Content-Length guard docstring to match its real behavior The docstring still described the Content-Length bail-out as a "broad, case-insensitive substring" scan over the whole header block -- accurate for the original implementation, but stale after the precision fix that narrowed it to an exact per-line field-name match (avoiding a false positive when an unrelated header value or the reason phrase merely contains the text "content-length"). Update the docstring to describe the actual check, including the resulting narrow gap it shares with the Transfer-Encoding match: a Content-Length header expressed only via obsolete line folding won't be detected. No behavior change. Full suite (2026 tests) still passes with 100% coverage; mypy strict and ruff are clean. --- src/httpcore2/httpcore2/_async/http11.py | 14 ++++++++++---- src/httpcore2/httpcore2/_sync/http11.py | 14 ++++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/httpcore2/httpcore2/_async/http11.py b/src/httpcore2/httpcore2/_async/http11.py index 91fb7eb0..f5b3ce4b 100644 --- a/src/httpcore2/httpcore2/_async/http11.py +++ b/src/httpcore2/httpcore2/_async/http11.py @@ -74,13 +74,19 @@ def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: Any other case (differing values, folded lines, malformed lines) is left completely untouched, so h11 still raises for it exactly as before. - Never applies if the header block also contains anything resembling a - `Content-Length` header (a deliberately broad, case-insensitive substring - check, not a precise parse). `Transfer-Encoding` combined with + Never applies if the header block also contains a `Content-Length` + header: every `\n`-split line's field name (the part before the first + `:`, matched case-insensitively, not stripped of whitespace -- same + reasoning as the `Transfer-Encoding` match above) is checked against + `content-length` exactly. `Transfer-Encoding` combined with `Content-Length` is exactly the shape of the classic conflicting-framing request-smuggling primitive that RFC 9112 requires treating as an error; issue #622's actual reproductions never combine the two, so giving up the - merge here costs nothing while closing off that class of ambiguity. + merge here costs nothing while closing off that class of ambiguity. Like + the `Transfer-Encoding` match, this doesn't account for obsolete line + folding, so a `Content-Length` header expressed only via a folded + continuation line won't be detected -- an accepted, narrow gap, since an + undetected fold is left untouched either way (see above). """ if any(line.partition(b":")[0].lower() == b"content-length" for line in header_block.split(b"\n")): return header_block diff --git a/src/httpcore2/httpcore2/_sync/http11.py b/src/httpcore2/httpcore2/_sync/http11.py index 14e29328..8e226083 100644 --- a/src/httpcore2/httpcore2/_sync/http11.py +++ b/src/httpcore2/httpcore2/_sync/http11.py @@ -74,13 +74,19 @@ def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: Any other case (differing values, folded lines, malformed lines) is left completely untouched, so h11 still raises for it exactly as before. - Never applies if the header block also contains anything resembling a - `Content-Length` header (a deliberately broad, case-insensitive substring - check, not a precise parse). `Transfer-Encoding` combined with + Never applies if the header block also contains a `Content-Length` + header: every `\n`-split line's field name (the part before the first + `:`, matched case-insensitively, not stripped of whitespace -- same + reasoning as the `Transfer-Encoding` match above) is checked against + `content-length` exactly. `Transfer-Encoding` combined with `Content-Length` is exactly the shape of the classic conflicting-framing request-smuggling primitive that RFC 9112 requires treating as an error; issue #622's actual reproductions never combine the two, so giving up the - merge here costs nothing while closing off that class of ambiguity. + merge here costs nothing while closing off that class of ambiguity. Like + the `Transfer-Encoding` match, this doesn't account for obsolete line + folding, so a `Content-Length` header expressed only via a folded + continuation line won't be detected -- an accepted, narrow gap, since an + undetected fold is left untouched either way (see above). """ if any(line.partition(b":")[0].lower() == b"content-length" for line in header_block.split(b"\n")): return header_block