diff --git a/src/httpcore2/httpcore2/_async/http11.py b/src/httpcore2/httpcore2/_async/http11.py index 2fc94452..f5b3ce4b 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,6 +39,106 @@ class HTTPConnectionState(enum.IntEnum): CLOSED = 3 +# 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") + + +def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: + """ + 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. + + 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. 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 + + 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] + + 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 + + if content[:1] in (b" ", b"\t"): + continue # obsolete-line-fold continuation of the previous line + + 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 + + name, sep, value = content.partition(b":") + if not (sep and name.lower() == b"transfer-encoding" and value.strip(b" \t").lower() == b"chunked"): + continue + + if seen_chunked_transfer_encoding: + delete_spans.append((span_start, span_end)) + else: + seen_chunked_transfer_encoding = True + + if not delete_spans: + return header_block + + 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): READ_NUM_BYTES = 64 * 1024 MAX_INCOMPLETE_EVENT_SIZE = 100 * 1024 @@ -59,6 +160,29 @@ 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`. 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 + # 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): @@ -176,7 +300,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 @@ -197,21 +330,81 @@ 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, self._response_header_search_from + ) + if match is not None: + 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 = 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 = 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. 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 50bce833..8e226083 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,6 +39,106 @@ class HTTPConnectionState(enum.IntEnum): CLOSED = 3 +# 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") + + +def _merge_duplicate_chunked_transfer_encoding(header_block: bytes) -> bytes: + """ + 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. + + 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. 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 + + 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] + + 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 + + if content[:1] in (b" ", b"\t"): + continue # obsolete-line-fold continuation of the previous line + + 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 + + name, sep, value = content.partition(b":") + if not (sep and name.lower() == b"transfer-encoding" and value.strip(b" \t").lower() == b"chunked"): + continue + + if seen_chunked_transfer_encoding: + delete_spans.append((span_start, span_end)) + else: + seen_chunked_transfer_encoding = True + + if not delete_spans: + return header_block + + 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): READ_NUM_BYTES = 64 * 1024 MAX_INCOMPLETE_EVENT_SIZE = 100 * 1024 @@ -59,6 +160,29 @@ 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`. 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 + # 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): @@ -176,7 +300,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 @@ -197,21 +330,81 @@ 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, self._response_header_search_from + ) + if match is not None: + 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 = 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 = 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. 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 d0087b19..0b685916 100644 --- a/tests/httpcore2/_async/test_http11.py +++ b/tests/httpcore2/_async/test_http11.py @@ -326,6 +326,293 @@ 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_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: + """ + 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_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_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 f886ef47..acddef80 100644 --- a/tests/httpcore2/_sync/test_http11.py +++ b/tests/httpcore2/_sync/test_http11.py @@ -327,6 +327,293 @@ 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_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 + 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_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_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.