mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
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.
This commit is contained in:
parent
10e4829201
commit
4cb33cd9e3
4 changed files with 69 additions and 0 deletions
|
|
@ -39,6 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
* **install:** stop leaking a file descriptor on every `headroom install start`. `start_detached_agent()` opened the agent log file and handed it to `subprocess.Popen` but never closed the parent's copy, so each call leaked one fd (and pinned the log file open against rotation). The parent now closes its copy in a `try/finally` once the child has inherited it — the close also runs if `Popen` raises ([#1554](https://github.com/headroomlabs-ai/headroom/issues/1554)).
|
||||
* **memory/sync:** stop the Codex AGENTS.md sync adapter from erasing previously-synced memories on every export. `sync_export` hands each adapter only the *delta* (memories the agent lacks), but `CodexAdapter.write_memories` rebuilt its whole managed section from just that delta — so each sync overwrote the section with only the new items, thrashing the file between disjoint subsets and never accumulating. It now merges the delta into the facts already present (deduped), matching the additive contract the ClaudeCode adapter already follows.
|
||||
* **cli/proxy:** honor `HEADROOM_MIN_TOKENS=0` / `HEADROOM_MAX_ITEMS=0`. The Click `proxy` command built these with `_get_env_int_optional(name) or 500`/`or 50`, so an explicit `0` — a legitimate value (`min_tokens_to_crush=0` means "crush every item") — was treated as falsy and silently replaced with the default. The `headroom proxy` argparse path already preserved `0` via `_get_env_int`, so the two entry points disagreed. The Click path now uses the same None-checking helper.
|
||||
* **proxy:** strip the inbound `Content-Encoding`/`Transfer-Encoding` request headers on the Anthropic `/v1/messages` and OpenAI `/v1/chat/completions` paths before forwarding upstream. `read_request_json_with_bytes` already decompresses the inbound body (zstd/gzip/deflate/br), so the bytes forwarded upstream are plain JSON — but these two handlers left the original `content-encoding` header in place, so a client (or an edge proxy like a Cloudflare Worker) that sent a compressed body got its request rejected with upstream HTTP 400 because the provider tried to decompress already-decoded JSON. The `/v1/responses` handler already carried this fix (#1542); it is now applied to the messages and chat paths too.
|
||||
* **proxy:** include the system prompt, tools, and the response-shaping request fields in the SemanticCache key. `_compute_key` hashed only `{model, messages}`, so two non-streaming requests with identical messages but a different top-level `system` prompt, tool set, sampling config, or output-shaping field collided on one key and the second caller was served the first's cached response — generated under different request semantics, in the default config (`cache_enabled` defaults on). The key now folds the request fields that shape generation — `temperature`/`top_p`/`top_k`/`max_tokens`/`stop`, plus OpenAI `tool_choice`/`response_format`/`parallel_tool_calls`/`seed`/`presence_penalty`/`frequency_penalty`/`logit_bias`/`n`/`logprobs`/`top_logprobs`/`reasoning_effort`/`verbosity`/`modalities` and Anthropic `thinking`/`tool_choice`/`output_config` — canonicalizing `system`/`tools` so a moved `cache_control` breakpoint does not fragment it, and the handlers snapshot the fields once at the cache read and reuse them at write so a body mutated by the pipeline cannot diverge the key. Non-streaming path only.
|
||||
* **learn (verbosity):** `--verbosity --apply --all` now aggregates the savings baseline across every project instead of overwriting it per project (last-project-wins), which previously left the output shaper with a tiny, unrepresentative baseline. The applied verbosity level comes from the project with the most samples ([#1288](https://github.com/headroomlabs-ai/headroom/pull/1288)).
|
||||
* **proxy/anthropic:** restore token-mode compression on continued Claude Code turns with a frozen prefix and deferred CCR tool injection. Token mode now runs request-side compression even when the client did not pre-register `headroom_retrieve`, relying on the existing marker-triggered injection override to keep emitted CCR markers redeemable ([#1487](https://github.com/headroomlabs-ai/headroom/issues/1487)).
|
||||
|
|
|
|||
|
|
@ -775,6 +775,13 @@ class AnthropicHandlerMixin:
|
|||
headers = dict(request.headers.items())
|
||||
headers.pop("host", None)
|
||||
headers.pop("content-length", None)
|
||||
# read_request_json_with_bytes already content-decoded the inbound
|
||||
# body (zstd/gzip/deflate/br), so the bytes we forward are plain.
|
||||
# A stale content-encoding makes the upstream try to decompress
|
||||
# already-decoded JSON and reject it with HTTP 400 (#1542 — same
|
||||
# fix the /v1/responses path already carries).
|
||||
headers.pop("content-encoding", None)
|
||||
headers.pop("transfer-encoding", None)
|
||||
# Strip accept-encoding so httpx negotiates its own encoding.
|
||||
# Edge proxies (Cloudflare Workers, etc.) may forward "br, zstd" which
|
||||
# the upstream can honor; if httpx lacks brotli support the response
|
||||
|
|
|
|||
|
|
@ -2204,6 +2204,12 @@ class OpenAIHandlerMixin:
|
|||
headers = dict(request.headers.items())
|
||||
headers.pop("host", None)
|
||||
headers.pop("content-length", None)
|
||||
# The parsed body was already content-decoded upstream, so the bytes we
|
||||
# forward are plain JSON. A stale content-encoding makes OpenAI try to
|
||||
# decompress already-decoded JSON and reject it with HTTP 400 (#1542 —
|
||||
# same fix the /v1/responses path already carries).
|
||||
headers.pop("content-encoding", None)
|
||||
headers.pop("transfer-encoding", None)
|
||||
# Strip accept-encoding so httpx negotiates its own encoding.
|
||||
# Cloudflare Workers forward "br, zstd" which OpenAI may honor;
|
||||
# if httpx lacks brotli support the response body is undecipherable → 502.
|
||||
|
|
|
|||
|
|
@ -195,6 +195,61 @@ class TestAcceptEncodingStripping:
|
|||
assert "accept-encoding" not in headers
|
||||
|
||||
|
||||
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."""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue