diff --git a/tests/e2e_cassette_proxy/addon.py b/tests/e2e_cassette_proxy/addon.py index 4a131a0eb4a..ed23c6bcc80 100644 --- a/tests/e2e_cassette_proxy/addon.py +++ b/tests/e2e_cassette_proxy/addon.py @@ -18,7 +18,8 @@ from __future__ import annotations import logging import os -from typing import Optional +import time +from typing import Optional, Tuple from mitmproxy import ctx, http # type: ignore[import-not-found] @@ -73,6 +74,54 @@ def _is_2xx(status_code: int) -> bool: return 200 <= status_code < 300 +def _to_header_bytes(value: str) -> bytes: + """Encode a header name or value back to bytes for mitmproxy. + + HTTP/1.1 header field names are 7-bit ASCII per RFC 7230 § 3.2; + values are also ASCII in practice but technically allow opaque + octets, which mitmproxy round-trips via the surrogateescape codec. + Latin-1 covers both cases without raising on stray high bytes that + might have leaked in via misbehaved upstreams. + """ + return value.encode("latin-1", errors="replace") + + +def _build_replay_response(cached: CachedResponse) -> http.Response: + """Build a mitmproxy ``Response`` from a cached payload. + + We bypass ``http.Response.make`` because: + + 1. ``make`` requires a ``Headers`` instance or a ``dict`` to do its + own bytes-conversion. Passing a list of ``(str, str)`` tuples + falls into its iterable branch which calls ``Headers(headers)`` + directly — and ``Headers`` raises ``TypeError: Header fields + must be bytes``. (This was the original bug: every cache hit + silently fell through to upstream because the addon raised here.) + 2. ``make`` calls ``Message.set_content`` which re-encodes the body + per the cached ``Content-Encoding`` header. Since we recorded + ``raw_content`` (i.e. the already-encoded wire bytes), going + through ``set_content`` would double-gzip those payloads. + + Constructing the ``Response`` directly lets us hand mitmproxy bytes + headers + the raw body in one shot, exactly mirroring what was on + the wire when we recorded it. + """ + fields: Tuple[Tuple[bytes, bytes], ...] = tuple( + (_to_header_bytes(k), _to_header_bytes(v)) for k, v in cached.headers + ) + now = time.time() + return http.Response( + http_version=b"HTTP/1.1", + status_code=cached.status_code, + reason=_to_header_bytes(cached.reason) or b"OK", + headers=fields, + content=cached.body, + trailers=None, + timestamp_start=now, + timestamp_end=now, + ) + + class CassetteAddon: """mitmproxy addon class. Created once at startup; ``request`` and ``response`` are invoked per flow.""" @@ -151,11 +200,7 @@ class CassetteAddon: f"status={cached.status_code} bytes={len(cached.body)}" ) self._stats["hit"] += 1 - flow.response = http.Response.make( - cached.status_code, - cached.body, - list(cached.headers), - ) + flow.response = _build_replay_response(cached) def response(self, flow: http.HTTPFlow) -> None: if self._store is None or self._should_skip(flow): @@ -172,7 +217,11 @@ class CassetteAddon: key = self._key_for(flow) cached = CachedResponse( status_code=flow.response.status_code, - headers=tuple((k, v) for k, v in flow.response.headers.items()), + # ``items(multi=True)`` preserves repeated headers (e.g. multiple + # ``Set-Cookie``) which the plain ``items()`` flattens into a + # single comma-joined string. Strings are fine on the wire to + # Redis; we re-encode to bytes on replay. + headers=tuple((k, v) for k, v in flow.response.headers.items(multi=True)), body=flow.response.raw_content or b"", reason=flow.response.reason or "", ) diff --git a/tests/test_litellm/e2e_cassette_proxy/test_addon.py b/tests/test_litellm/e2e_cassette_proxy/test_addon.py index 9c251e42224..96d43e270af 100644 --- a/tests/test_litellm/e2e_cassette_proxy/test_addon.py +++ b/tests/test_litellm/e2e_cassette_proxy/test_addon.py @@ -21,6 +21,17 @@ sys.path.insert( 0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "..")) ) +# Prefer the *real* mitmproxy if it's installed: that way the contract +# tests in ``test_replay_response_contract.py`` exercise the same +# objects this file does, and any future skew between our fake and +# real mitmproxy can't silently hide regressions. Fall through to the +# fake stub when mitmproxy isn't available (the typical unit-test env). +try: + import mitmproxy # noqa: F401 pre-load so setdefault below is a no-op + import mitmproxy.http # noqa: F401 +except ImportError: + pass + # Install a fake `mitmproxy` package *before* importing the addon, so the # real mitmproxy doesn't need to be present in the unit-test env. _mitmproxy_pkg = types.ModuleType("mitmproxy") @@ -28,16 +39,64 @@ _http_mod = types.ModuleType("mitmproxy.http") class _FakeHeaders(dict): - def items(self): + def items(self, multi: bool = False): + # Real mitmproxy ``Headers.items(multi=True)`` would return one + # tuple per repeated header (e.g. multiple ``Set-Cookie``). The + # fake doesn't model duplicates, so the kwarg is a no-op — we + # just need to accept it without ``TypeError``. return list(super().items()) class _FakeResponse: - def __init__(self, status_code, body=b"", headers=None, reason=""): + """Fake mitmproxy ``http.Response`` that accepts both the real + constructor signature (used by the addon's replay path) and a + simplified positional signature (used by these tests when faking an + upstream response). + """ + + def __init__( + self, + *args, + status_code=None, + body=None, + headers=None, + reason="", + # real-mitmproxy kwargs (the addon passes these via _build_replay_response) + http_version=b"HTTP/1.1", + content=None, + trailers=None, + timestamp_start=None, + timestamp_end=None, + ): + # Positional support: _FakeResponse(status_code, body=..., headers=...) + if args: + if status_code is None: + status_code = args[0] + if len(args) > 1 and body is None: + body = args[1] + # ``content`` (real mitmproxy) and ``body`` (test shorthand) are + # the same field — raw_content. + if body is None: + body = content if content is not None else b"" + if isinstance(headers, list): + # Bytes-tuple list (real mitmproxy contract) → str dict for + # the fake's case-insensitive lookups. + headers = { + (k.decode("latin-1") if isinstance(k, bytes) else k): ( + v.decode("latin-1") if isinstance(v, bytes) else v + ) + for k, v in headers + } + if isinstance(reason, bytes): + reason = reason.decode("latin-1") self.status_code = status_code self.raw_content = body self.headers = _FakeHeaders(headers or {}) self.reason = reason + self.http_version = http_version + self.trailers = trailers + self.timestamp_start = timestamp_start + self.timestamp_end = timestamp_end @classmethod def make(cls, status_code, body=b"", headers=None): diff --git a/tests/test_litellm/e2e_cassette_proxy/test_replay_response_contract.py b/tests/test_litellm/e2e_cassette_proxy/test_replay_response_contract.py new file mode 100644 index 00000000000..60c36020204 --- /dev/null +++ b/tests/test_litellm/e2e_cassette_proxy/test_replay_response_contract.py @@ -0,0 +1,146 @@ +"""Contract tests against real mitmproxy. + +The other addon unit tests stub out ``mitmproxy.http`` so they can run +without mitmproxy installed (it's a CI-only dependency installed via +``uv tool install`` rather than declared in ``pyproject.toml``). + +That stubbing once hid a real production bug: the addon called +``http.Response.make(status, body, list[(str, str)])`` which the fake +accepted, but real mitmproxy 11 raises ``TypeError: Header fields must +be bytes``. Every cache hit silently fell through to the upstream +because the addon raised inside ``request()``. + +This file exercises the *real* mitmproxy ``Response`` contract for the +replay path, so any future regression is caught locally instead of only +showing up as ``Addon error: ...`` in CI logs while tests pass. +""" + +from __future__ import annotations + +import pytest + +mitmproxy_http = pytest.importorskip("mitmproxy.http") + +# ``test_addon.py`` (collected alongside this file) installs a stubbed +# ``mitmproxy`` package into ``sys.modules`` so it can run without the +# real dependency. ``importorskip`` will happily return that stub and +# defeat the whole point of *this* file. Detect the stub by checking +# whether the loaded ``Headers`` enforces bytes the way real mitmproxy +# does, and skip if it doesn't. +try: + mitmproxy_http.Headers([("not", "bytes")]) # type: ignore[arg-type] +except TypeError: + pass # real mitmproxy — proceed +except Exception: + pytest.skip( + "real mitmproxy not available (fake stub detected); " + "install ``mitmproxy==11.0.2`` to run these contract tests", + allow_module_level=True, + ) +else: + pytest.skip( + "real mitmproxy not available (fake stub detected); " + "install ``mitmproxy==11.0.2`` to run these contract tests", + allow_module_level=True, + ) + +from tests.e2e_cassette_proxy.addon import _build_replay_response # noqa: E402 +from tests.e2e_cassette_proxy.redis_store import CachedResponse # noqa: E402 + + +def test_replay_response_accepts_string_headers_and_emits_bytes(): + """Cached headers are stored as ``str`` (msgpack roundtrip); the + replay builder must encode them back to ``bytes`` because mitmproxy + 11's ``Headers`` rejects anything else.""" + cached = CachedResponse( + status_code=200, + headers=( + ("Content-Type", "application/json"), + ("X-Trace-Id", "abc123"), + ), + body=b'{"id":"chatcmpl-1"}', + reason="OK", + ) + + response = _build_replay_response(cached) + + assert isinstance(response, mitmproxy_http.Response) + assert response.status_code == 200 + # mitmproxy's ``Headers`` decodes back to str on read; the underlying + # ``fields`` are bytes — verify both directions. + assert response.headers["Content-Type"] == "application/json" + assert response.headers["X-Trace-Id"] == "abc123" + for name, value in response.headers.fields: + assert isinstance(name, bytes), name + assert isinstance(value, bytes), value + + +def test_replay_response_preserves_raw_content_without_double_encoding(): + """When the upstream replied with ``Content-Encoding: gzip`` we recorded + the *gzipped* bytes (i.e. ``flow.response.raw_content``). On replay the + body must be set as ``raw_content`` so mitmproxy serves the same wire + payload — going through ``set_content`` would gzip-encode an already- + gzipped blob.""" + gzipped_body = ( + b"\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\x03\xab\xe6" + b"R\xa8\xe5\x02\x00\x9b\xae\x10\xee\x05\x00\x00\x00" + ) # arbitrary gzip-magic-prefixed bytes + cached = CachedResponse( + status_code=200, + headers=( + ("Content-Type", "application/json"), + ("Content-Encoding", "gzip"), + ("Content-Length", str(len(gzipped_body))), + ), + body=gzipped_body, + reason="OK", + ) + + response = _build_replay_response(cached) + + assert response.raw_content == gzipped_body, ( + "raw_content must be byte-identical to what we recorded; " + "going through set_content would re-gzip and corrupt it" + ) + + +def test_replay_response_handles_repeated_set_cookie_headers(): + """``items(multi=True)`` is used on the recording side specifically so + multi-value headers like ``Set-Cookie`` aren't silently merged into a + comma-joined string (which is RFC 7230 illegal for ``Set-Cookie``).""" + cached = CachedResponse( + status_code=200, + headers=( + ("Set-Cookie", "session=abc; Path=/"), + ("Set-Cookie", "csrf=xyz; Path=/"), + ("Content-Type", "text/html"), + ), + body=b"", + reason="OK", + ) + + response = _build_replay_response(cached) + + set_cookies = [ + v.decode("latin-1") + for k, v in response.headers.fields + if k.lower() == b"set-cookie" + ] + assert set_cookies == ["session=abc; Path=/", "csrf=xyz; Path=/"] + + +def test_replay_response_tolerates_empty_reason_phrase(): + cached = CachedResponse( + status_code=204, + headers=(), + body=b"", + reason="", + ) + + response = _build_replay_response(cached) + + assert response.status_code == 204 + # mitmproxy expects bytes; an empty reason should default to ``b"OK"`` + # at the wire level so the response is still serializable by + # mitmproxy's HTTP/1.1 codec. ``reason`` decodes back to str on read. + assert response.reason == "OK"