headroom/tests/test_proxy_compression_headers.py

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

284 lines
11 KiB
Python
Raw Permalink Normal View History

"""Tests for compression header handling in the proxy server.
These tests verify that the proxy correctly removes Content-Encoding headers
from responses after httpx automatically decompresses them, preventing
double-decompression errors (ZlibError) in clients.
"""
import gzip
import json
import pytest
@pytest.fixture
def mock_anthropic_response_with_compression_headers():
"""Create a mock response that simulates httpx behavior.
httpx automatically decompresses responses but leaves compression headers.
This is what causes the ZlibError bug we're testing for.
"""
class MockResponse:
"""Mock httpx response with compression headers."""
def __init__(self):
self.response_data = {
"id": "msg_test123",
"type": "message",
"role": "assistant",
"content": [{"type": "text", "text": "Hello!"}],
"model": "claude-3-5-sonnet-20241022",
"stop_reason": "end_turn",
"usage": {"input_tokens": 10, "output_tokens": 5},
}
# Body is already decompressed (httpx does this automatically)
self.content = json.dumps(self.response_data).encode("utf-8")
self.status_code = 200
# Headers still contain compression info (this is the bug!)
self.headers = {
"content-type": "application/json",
"content-encoding": "gzip", # Should be removed!
"content-length": str(len(gzip.compress(self.content))), # Wrong!
"x-request-id": "test-request-id",
}
return MockResponse()
class TestCompressionHeaderRemoval:
"""Tests for Content-Encoding header removal logic."""
def test_compression_headers_are_removed_from_dict(
self, mock_anthropic_response_with_compression_headers
):
"""Test that our fix removes compression headers from response headers."""
mock_response = mock_anthropic_response_with_compression_headers
# Simulate what the fixed code does
response_headers = dict(mock_response.headers)
response_headers.pop("content-encoding", None)
response_headers.pop("content-length", None)
# Verify compression headers are removed
assert "content-encoding" not in response_headers
assert "content-length" not in response_headers
# Verify other headers are preserved
assert response_headers["content-type"] == "application/json"
assert response_headers["x-request-id"] == "test-request-id"
def test_response_body_is_decompressed_not_compressed(
self, mock_anthropic_response_with_compression_headers
):
"""Verify the response content is already decompressed (httpx behavior)."""
mock_response = mock_anthropic_response_with_compression_headers
# The content should be valid JSON (decompressed)
response_data = json.loads(mock_response.content)
assert response_data["id"] == "msg_test123"
# Trying to decompress it again should fail (proving it's not compressed)
with pytest.raises((gzip.BadGzipFile, OSError, Exception)):
gzip.decompress(mock_response.content)
def test_headers_with_wrong_content_length_cause_issues(
self, mock_anthropic_response_with_compression_headers
):
"""Demonstrate that keeping compression headers causes length mismatch."""
mock_response = mock_anthropic_response_with_compression_headers
# The content-length header says the body is compressed size
claimed_length = int(mock_response.headers["content-length"])
# But the actual content is decompressed size
actual_length = len(mock_response.content)
# They don't match! This can cause client issues
assert claimed_length != actual_length
assert claimed_length < actual_length # Compressed is smaller
def test_removing_headers_fixes_length_mismatch(
self, mock_anthropic_response_with_compression_headers
):
"""Show that removing compression headers allows proper content-length."""
mock_response = mock_anthropic_response_with_compression_headers
# Apply the fix
response_headers = dict(mock_response.headers)
response_headers.pop("content-encoding", None)
response_headers.pop("content-length", None)
# Now we can set correct content-length
response_headers["content-length"] = str(len(mock_response.content))
# Verify it matches actual content
assert int(response_headers["content-length"]) == len(mock_response.content)
class TestAcceptEncodingStripping:
"""Tests for accept-encoding removal from forwarded request headers.
Edge proxies like Cloudflare Workers add accept-encoding values (e.g. br,
zstd) that the upstream provider may honor. If httpx lacks the matching
decompression library (e.g. brotli) it cannot decode the response body,
causing a UnicodeDecodeError and a 502 returned to the client.
The fix strips accept-encoding before forwarding so httpx negotiates its
own encoding independently.
"""
def test_accept_encoding_is_stripped_from_forwarded_headers(self):
"""accept-encoding must be removed before forwarding to the upstream."""
# Simulate headers as received from a Cloudflare Worker client
request_headers = {
"authorization": "Bearer sk-test",
"content-type": "application/json",
"accept-encoding": "gzip, br, zstd",
"host": "headroom.example.com",
"content-length": "123",
}
# Replicate the handler logic
headers = dict(request_headers.items())
headers.pop("host", None)
headers.pop("content-length", None)
headers.pop("accept-encoding", None)
assert "accept-encoding" not in headers
def test_other_headers_preserved_after_stripping(self):
"""Only hop-by-hop / negotiation headers are removed; auth etc. survive."""
request_headers = {
"authorization": "Bearer sk-test",
"content-type": "application/json",
"accept-encoding": "gzip, br",
"x-custom": "value",
"host": "headroom.example.com",
"content-length": "42",
}
headers = dict(request_headers.items())
headers.pop("host", None)
headers.pop("content-length", None)
headers.pop("accept-encoding", None)
assert headers["authorization"] == "Bearer sk-test"
assert headers["content-type"] == "application/json"
assert headers["x-custom"] == "value"
assert "host" not in headers
assert "content-length" not in headers
assert "accept-encoding" not in headers
def test_strip_is_safe_when_accept_encoding_absent(self):
"""pop() on a missing key must not raise — direct curl calls have no header."""
request_headers = {
"authorization": "Bearer sk-test",
"content-type": "application/json",
}
headers = dict(request_headers.items())
# Must not raise KeyError
headers.pop("accept-encoding", None)
assert headers == {
"authorization": "Bearer sk-test",
"content-type": "application/json",
}
def test_brotli_encoding_value_is_stripped(self):
"""Specifically guard against 'br' which breaks httpx without brotli package."""
for encoding_value in ["br", "gzip, br", "gzip, br, zstd", "zstd"]:
headers = {"accept-encoding": encoding_value, "content-type": "application/json"}
headers.pop("accept-encoding", None)
assert "accept-encoding" not in headers
fix(proxy): strip inbound Content-Encoding on messages/chat forward (#1970) ## Description The Anthropic `/v1/messages` and OpenAI `/v1/chat/completions` handlers decode the inbound request body before forwarding it upstream. `read_request_json_with_bytes` (helpers.py) inflates `zstd`/`gzip`/`deflate`/`br` bodies, and the handler forwards the resulting plain JSON. But both handlers build the upstream-bound header dict and pop only `host`, `content-length`, and `accept-encoding` — they leave the original `Content-Encoding` header in place: ```python headers = dict(request.headers.items()) headers.pop("host", None) headers.pop("content-length", None) headers.pop("accept-encoding", None) # content-encoding NOT popped ``` So when a client — or an edge proxy like a Cloudflare Worker — sends a request with `Content-Encoding: gzip` (or `zstd`/`br`/`deflate`) and a compressed body, Headroom decompresses it, then forwards plain JSON that still advertises `content-encoding: gzip`. The upstream provider tries to gunzip already-decoded JSON and rejects the request with HTTP 400. Every such request fails. This is a known class of bug: the `/v1/responses` handler already fixes exactly this at `openai.py` with the comment *"Leaving a stale content-encoding header makes the upstream try to decompress already-decoded JSON and reject it with HTTP 400 (#1542)."* That fix landed only on the `/responses` path — the messages and chat paths were missed. Closes: no issue filed — found while auditing request-header forwarding across the handlers. ## Fix Pop `content-encoding` and `transfer-encoding` in both handlers, right after the existing `content-length` pop, mirroring the `/v1/responses` handler: ```python headers.pop("content-encoding", None) headers.pop("transfer-encoding", None) ``` ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/proxy/handlers/anthropic.py`: strip `content-encoding`/`transfer-encoding` from the upstream-bound request headers in `handle_anthropic_messages`. - `headroom/proxy/handlers/openai.py`: same strip in the `/v1/chat/completions` handler. - `tests/test_proxy_compression_headers.py`: add `TestRequestContentEncodingStripping` covering gzip/zstd/deflate/br, `transfer-encoding`, and the absent-header (plain curl) case. ## Testing - [x] New regression tests added (`tests/test_proxy_compression_headers.py`) - [x] Linting passes (`ruff check`) and formatting is clean (`ruff format --check`) - [ ] Full `pytest` deferred to CI (local-OOM reason below). ```text $ uv run ruff check headroom/proxy/handlers/anthropic.py headroom/proxy/handlers/openai.py tests/test_proxy_compression_headers.py All checks passed! ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.10, headroom from this branch. Importing `headroom` pulls in the torch/transformers stack and a full `pytest` gets OOM-killed on this box, so I verified the header logic with a dependency-free script (the same pure-dict pattern the existing tests in this file use) and left the full pytest to CI. - Exact command / steps: replicated the handler's request-header stripping (old vs new) in a standalone script and ran a `gzip`/`zstd`/`deflate`/`br` request through both. - Observed result: the old logic keeps `content-encoding` (which is what makes the upstream 400); the new logic strips it while preserving `authorization` and `content-type`: ```text OK gzip: old leaks 'gzip' -> upstream 400 ; new strips it OK zstd: old leaks 'zstd' -> upstream 400 ; new strips it OK deflate: old leaks 'deflate' -> upstream 400 ; new strips it OK br: old leaks 'br' -> upstream 400 ; new strips it OK transfer-encoding stripped OK safe when absent CONTENT-ENCODING STRIP VERIFIED ``` - Not tested: an end-to-end POST of a real gzip body through a booted proxy to a live upstream (needs the heavy stack + a provider key). The header now matches the byte-faithful forwarding the `/responses` path already does, and the new tests exercise the exact strip logic. Full local `pytest` deferred to CI (OOM, per above). ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes — ran lint + a standalone logic check; full pytest deferred to CI (local OOM, disclosed above) - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes - Small, contained parity fix — two `pop()` calls plus tests, no new dependencies. - @JerrettDavis tagging you since you've been triaging the proxy-forwarding fixes (this is the sibling of the #1542 `/responses` fix) — should be a quick one if you have a moment.
2026-07-10 21:15:58 +05:30
class TestRequestContentEncodingStripping:
"""Tests for content-encoding removal from forwarded *request* headers.
read_request_json_with_bytes decompresses the inbound body (zstd/gzip/
deflate/br) before the handler forwards it, so the bytes sent upstream are
plain JSON. If the original Content-Encoding header rides along, the
upstream tries to decompress already-decoded JSON and rejects it with HTTP
400 (#1542). The /v1/responses handler stripped it; the Anthropic messages
and OpenAI chat handlers must do the same.
"""
def _strip(self, request_headers: dict[str, str]) -> dict[str, str]:
"""Replicate the fixed handler request-header logic."""
headers = dict(request_headers.items())
headers.pop("host", None)
headers.pop("content-length", None)
headers.pop("content-encoding", None)
headers.pop("transfer-encoding", None)
headers.pop("accept-encoding", None)
return headers
@pytest.mark.parametrize("encoding", ["gzip", "zstd", "deflate", "br"])
def test_content_encoding_is_stripped_from_forwarded_request(self, encoding):
"""A compressed inbound request must not forward its content-encoding."""
request_headers = {
"authorization": "Bearer sk-test",
"content-type": "application/json",
"content-encoding": encoding,
"content-length": "123",
"host": "headroom.example.com",
}
headers = self._strip(request_headers)
assert "content-encoding" not in headers
# Auth and content-type survive so the upstream still routes/parses it.
assert headers["authorization"] == "Bearer sk-test"
assert headers["content-type"] == "application/json"
def test_transfer_encoding_is_stripped(self):
"""transfer-encoding: chunked also describes the wire body, not the payload."""
headers = self._strip({"transfer-encoding": "chunked", "content-type": "application/json"})
assert "transfer-encoding" not in headers
def test_strip_is_safe_when_content_encoding_absent(self):
"""A plain curl request has no content-encoding — pop must not raise."""
headers = self._strip(
{"authorization": "Bearer sk-test", "content-type": "application/json"}
)
assert headers == {
"authorization": "Bearer sk-test",
"content-type": "application/json",
}
class TestNoRegressionForUncompressedResponses:
"""Ensure the fix doesn't break responses that were never compressed."""
def test_pop_on_missing_keys_is_safe(self):
"""Verify that .pop() on non-existent keys doesn't cause errors."""
headers = {
"content-type": "application/json",
# No compression headers
}
# This should not raise KeyError
headers.pop("content-encoding", None)
headers.pop("content-length", None)
# Headers should be unchanged
assert headers == {"content-type": "application/json"}
def test_dict_conversion_preserves_headers(self):
"""Verify dict() conversion doesn't lose headers."""
original_headers = {
"content-type": "application/json",
"x-custom-header": "value",
"authorization": "Bearer token",
}
# Convert to dict (as the fix does)
converted = dict(original_headers)
# All headers preserved
assert converted == original_headers
assert converted is not original_headers # New object