Skip to content

Commit 8369b7f

Browse files
committed
fix(proxy/streaming): tolerate malformed upstream SSE events
## Description Three parsers read upstream-controlled SSE bodies with `json.loads` and then reach into the result without checking its shape: `_parse_sse_usage`, `_parse_sse_usage_from_buffer` and `_parse_sse_to_response`. A valid-JSON value of the wrong shape passes the `json.JSONDecodeError` guard and then raises. Three distinct ways: - a non-object event (`["x"]`, `"str"`, `42`) makes `.get` raise `AttributeError` - an explicit `"key": null` bypasses a `.get(key, {})` default, because the default applies only to a *missing* key, not a present-and-null one - an unhashable `index` (a list or dict) raises `TypeError: unhashable type` when used to key the block map In `_parse_sse_to_response` the raise escapes into `_finalize_stream_response` and tears down a stream the client is already reading. Several call sites reach it from a bare `finally`, so there is no handler above it at all. **This is a robustness invariant, not a bug report about any specific upstream.** I am not claiming Anthropic emits these shapes; I have no evidence of that. The Anthropic handler serves any Anthropic-shaped upstream (`--backend anthropic|bedrock|openrouter|anyllm|litellm-<provider>`, `ANTHROPIC_TARGET_API_URL`), so Bedrock, OpenRouter, LiteLLM, vLLM and Vertex all flow through these parsers. A parser on a proxy's critical streaming path should not raise on a well-formed-JSON frame of unexpected shape, whatever the source. The guard costs one `isinstance`; being wrong costs the user's session. **Scope: frame shape only.** Value validity — a token count that is a string, `Infinity`, or a >4300-digit integer literal — is a separate defect class that reaches the same parsers and is deliberately left for a follow-up. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - **headroom/proxy/handlers/streaming.py** - Add three shared normalizers — `_sse_dict`, `_sse_str`, `_sse_index` — so every upstream-derived container is shape-checked with one idiom instead of a mix of inline `isinstance` and `.get(..., {})` defaults. - `_sse_index` rejects `bool` as well as non-`int`, so `true` cannot silently alias block index `1`, and unhashable values cannot reach the block map. - Guard the positions the earlier pass missed: `content_block_delta.delta`, `message_delta.delta`, `message_delta.usage` (`dict.update` on a non-mapping raises — the `message_start` twin was already guarded, this sibling was not), the `index` on all three `content_block_*` events, and the string accumulators for `text` / `partial_json` / `thinking`. - Guard `_parse_sse_usage`, which had no shape checks at all — not on the top-level event, nor on the anthropic, openai or gemini branches. - Guard the gemini `usageMetadata` branch in `_parse_sse_usage_from_buffer`, which used a truthiness check where its two sibling branches used `isinstance`. - Stop the non-standard-block copy-through from seeding the parser's own scratch keys (`_partial_json`, `thinking_buffer`) via `_BLOCK_SCRATCH_KEYS`, and tolerate a non-list `citations` that a copy-through already placed. - **tests/test_streaming_sse_malformed_events.py** — new file, 408 cases. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check`, `ruff format`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added The sweep is enumerated, not randomized: the failure surface is finite, so the cases can be exact and no `hypothesis` dependency is needed. This matches the existing precedent in `tests/test_toin_observation_only.py`. Happy-path fixtures are transcribed from real `api.anthropic.com` streams rather than hand-written, which corrected four wrong assumptions in the previous fixtures: `message_delta.usage` repeats the *full* usage block (not just `output_tokens`); `message_start.message` carries `stop_details: null` alongside the other two nulls; `usage.cache_creation` is always present as a nested object and is what `_extract_anthropic_cache_ttl_metrics` reads; and a `tool_use` block carries a non-standard `caller` field. ### Test Output ```text $ pytest tests/test_streaming_sse_malformed_events.py 408 passed # same file against origin/main's parsers 191 failed, 217 passed # regression sweep: all 22 test files touching SSE parsing 710 passed, 5 skipped $ mypy headroom --ignore-missing-imports Success: no issues found in 509 source files ``` ## Real Behavior Proof - **Environment:** macOS 15 (Darwin 25.4.0, arm64), Python 3.13.7, Rust 1.95.0 per `rust-toolchain.toml`, `uv sync --extra dev`. - **Real upstream capture:** three live streaming `POST /v1/messages` calls to `api.anthropic.com` (`claude-haiku-4-5`) — a text stream, a `tool_use` stream and an extended-thinking stream — captured as raw wire bytes and used to build the happy-path fixtures. This is what surfaced the four fixture errors above. - **Differential:** the 408-case file was run against `origin/main`'s parsers via `git show origin/main:...` (not `git stash`, which silently no-ops once the fix is committed and produces a false pass). 191 cases fail there and pass here. - **Exhaustive shape fuzz:** 700 generated cases crossing every upstream-derived container position in the three parsers with `None`, `"str"`, `42`, `3.5`, `True`, `False`, `["x"]`, `[]`, `{}`, `{"k":"v"}`. Zero raises after the change; before it, the positions listed under Changes Made all raise. - **Per-position legitimacy check:** each injection position in the sweep was confirmed to produce at least one genuine failure against `origin/main`, so no case passes vacuously. This caught four delta-shaped tests that were injecting their fault before any content block had opened, where the parser skips the body and the guard is never reached. - **Not tested:** - **No evidence these shapes occur in the wild.** The fault shapes are derived from the parsers' own unguarded access positions, not observed in real upstream traffic. I searched the tracker for this crash signature and both function names and found no report. The claim is survivability, not incidence. - The real API cannot be made to emit malformed frames on demand, so the fault injection is synthetic; only the happy-path fixtures come from live traffic. - No live third-party gateway (Bedrock/OpenRouter/LiteLLM/Vertex) exercised, despite those being the motivating case. - No WebSocket path, no Windows, not run against a live Claude Code session. - Value-validity faults are out of scope and still raise; see Additional Notes. ## Additional Notes Two findings left deliberately unfixed, both pre-existing and both outside the frame-shape scope of this change: 1. **Value validity.** `_extract_anthropic_cache_ttl_metrics` calls `int()` on an upstream value with no `try`; `_usage_int` does not catch `OverflowError`, and `json.loads` accepts bare `Infinity`; `except json.JSONDecodeError` is too narrow for a >4300-digit integer literal (plain `ValueError`) or deep nesting (`RecursionError`); and the anthropic and gemini branches export non-`int` token values out of a `dict[str, int]`-annotated function. 2. **Reconstruction fidelity.** `content_block_start` copies through unknown fields only for non-standard block types, so the real `caller` field on a `tool_use` block is dropped on reconstruction. `test_real_tool_use_stream_reconstructs_tool_input` pins the current behaviour rather than asserting the desired one. ## 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 — n/a, no user-facing surface changed - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective - [x] New and existing unit tests pass locally with my changes - [x] I did **not** edit `CHANGELOG.md` No new dependencies added.
1 parent cb8f4b6 commit 8369b7f

2 files changed

Lines changed: 710 additions & 31 deletions

File tree

headroom/proxy/handlers/streaming.py

Lines changed: 99 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,49 @@
3030
logger = logging.getLogger("headroom.proxy")
3131

3232

33+
# Upstream controls every SSE event body, so the shape of anything reached
34+
# through `json.loads` is not guaranteed. These normalize a wrong-shaped value
35+
# to a harmless one instead of letting it raise on the streaming path, where
36+
# the raise escapes into `_finalize_stream_response` and tears down a stream
37+
# the client is already reading. They guard *shape*, not value validity.
38+
39+
40+
def _sse_dict(value: Any) -> dict[str, Any]:
41+
"""Upstream-supplied object, or ``{}`` when it is not one.
42+
43+
An explicit ``"key": null`` bypasses a ``.get(key, {})`` default — the
44+
default only applies to a *missing* key — so nested objects are
45+
type-checked rather than defaulted.
46+
"""
47+
return value if isinstance(value, dict) else {}
48+
49+
50+
def _sse_str(value: Any) -> str:
51+
"""Upstream-supplied text, or ``""`` when it is not a string.
52+
53+
Accumulating a non-string delta would raise ``TypeError`` on ``+``.
54+
"""
55+
return value if isinstance(value, str) else ""
56+
57+
58+
def _sse_index(value: Any, default: int | None = None) -> int | None:
59+
"""Upstream-supplied block index, or ``default`` when unusable as a key.
60+
61+
``index`` keys ``blocks_by_index`` and ``appended_block_keys``; a list or
62+
dict there raises ``TypeError: unhashable type``. ``bool`` is rejected too
63+
so ``true`` cannot silently alias index ``1``.
64+
"""
65+
if isinstance(value, bool) or not isinstance(value, int):
66+
return default
67+
return value
68+
69+
70+
# Keys `_parse_sse_to_response` owns on its in-progress block dicts. Upstream
71+
# must not seed them via the non-standard-block copy-through, or a non-string
72+
# would reach the delta accumulators. `type` is set explicitly, not copied.
73+
_BLOCK_SCRATCH_KEYS = frozenset({"type", "_partial_json", "thinking_buffer"})
74+
75+
3376
def _parse_completion_tokens_from_sse_chunk(chunk_bytes: bytes) -> int | None:
3477
"""Extract `usage.completion_tokens` from a single SSE chunk if present.
3578
@@ -179,6 +222,9 @@ def _parse_sse_usage(self, chunk: bytes, provider: str) -> dict[str, int] | None
179222
except json.JSONDecodeError:
180223
continue
181224

225+
if not isinstance(data, dict):
226+
continue
227+
182228
usage = {}
183229

184230
if provider == "anthropic":
@@ -187,8 +233,7 @@ def _parse_sse_usage(self, chunk: bytes, provider: str) -> dict[str, int] | None
187233
event_type = data.get("type", "")
188234

189235
if event_type == "message_start":
190-
msg = data.get("message", {})
191-
msg_usage = msg.get("usage", {})
236+
msg_usage = _sse_dict(_sse_dict(data.get("message")).get("usage"))
192237
if msg_usage:
193238
usage["input_tokens"] = msg_usage.get("input_tokens", 0)
194239
usage["cache_read_input_tokens"] = msg_usage.get(
@@ -204,24 +249,24 @@ def _parse_sse_usage(self, chunk: bytes, provider: str) -> dict[str, int] | None
204249
usage["cache_creation_ephemeral_1h_input_tokens"] = cache_write_1h
205250

206251
elif event_type == "message_delta":
207-
delta_usage = data.get("usage", {})
252+
delta_usage = _sse_dict(data.get("usage"))
208253
if delta_usage:
209254
usage["output_tokens"] = delta_usage.get("output_tokens", 0)
210255

211256
elif provider == "openai":
212257
# OpenAI sends usage in final chunk (when stream_options.include_usage=true)
213-
chunk_usage = data.get("usage")
258+
chunk_usage = _sse_dict(data.get("usage"))
214259
if chunk_usage:
215260
usage["input_tokens"] = chunk_usage.get("prompt_tokens", 0)
216261
usage["output_tokens"] = chunk_usage.get("completion_tokens", 0)
217262
# OpenAI has cached tokens in prompt_tokens_details
218-
details = chunk_usage.get("prompt_tokens_details") or {}
263+
details = _sse_dict(chunk_usage.get("prompt_tokens_details"))
219264
usage["cache_read_input_tokens"] = details.get("cached_tokens", 0)
220265

221266
elif provider == "gemini":
222267
# Gemini sends usageMetadata in each streaming chunk
223268
# Format: {"usageMetadata": {"promptTokenCount": N, "candidatesTokenCount": M}}
224-
usage_meta = data.get("usageMetadata")
269+
usage_meta = _sse_dict(data.get("usageMetadata"))
225270
if usage_meta:
226271
usage["input_tokens"] = usage_meta.get("promptTokenCount", 0)
227272
usage["output_tokens"] = usage_meta.get("candidatesTokenCount", 0)
@@ -275,11 +320,18 @@ def _parse_sse_usage_from_buffer(
275320
except json.JSONDecodeError:
276321
continue
277322

323+
# The upstream controls the event body. A valid-JSON non-object
324+
# (a bare array or string, e.g. from an overloaded frontend's
325+
# error page framed as SSE) makes `.get` raise AttributeError,
326+
# which the JSONDecodeError guard above does not catch. Skip it
327+
# like any other unusable event.
328+
if not isinstance(data, dict):
329+
continue
330+
278331
if provider == "anthropic":
279332
event_type = data.get("type", "")
280333
if event_type == "message_start":
281-
msg = data.get("message", {})
282-
msg_usage = msg.get("usage", {})
334+
msg_usage = _sse_dict(_sse_dict(data.get("message")).get("usage"))
283335
if msg_usage:
284336
usage_found["input_tokens"] = msg_usage.get("input_tokens", 0)
285337
usage_found["cache_read_input_tokens"] = msg_usage.get(
@@ -299,7 +351,7 @@ def _parse_sse_usage_from_buffer(
299351
f"cache_write={usage_found.get('cache_creation_input_tokens')}"
300352
)
301353
elif event_type == "message_delta":
302-
delta_usage = data.get("usage", {})
354+
delta_usage = _sse_dict(data.get("usage"))
303355
if delta_usage:
304356
usage_found["output_tokens"] = delta_usage.get("output_tokens", 0)
305357

@@ -339,7 +391,7 @@ def _usage_int(value: Any) -> int:
339391
)
340392

341393
elif provider == "gemini":
342-
usage_meta = data.get("usageMetadata")
394+
usage_meta = _sse_dict(data.get("usageMetadata"))
343395
if usage_meta:
344396
usage_found["input_tokens"] = usage_meta.get("promptTokenCount", 0)
345397
usage_found["output_tokens"] = usage_meta.get("candidatesTokenCount", 0)
@@ -402,29 +454,36 @@ def _parse_sse_to_response(self, sse_data: str, provider: str) -> dict[str, Any]
402454
except json.JSONDecodeError:
403455
continue
404456

457+
# Same upstream-controlled shape guard as
458+
# ``_parse_sse_usage_from_buffer``: a valid-JSON non-object event
459+
# would raise AttributeError below, and here that escapes into
460+
# ``_finalize_stream_response`` and tears down a stream the client
461+
# was already reading.
462+
if not isinstance(data, dict):
463+
continue
464+
405465
event_type = data.get("type", "")
406466

407467
if event_type == "message_start":
408-
msg = data.get("message", {})
468+
msg = _sse_dict(data.get("message"))
409469
response["id"] = msg.get("id")
410470
response["model"] = msg.get("model")
411471
response["role"] = msg.get("role", "assistant")
412472
response["stop_reason"] = msg.get("stop_reason")
413473
if "stop_details" in msg:
414474
response["stop_details"] = msg["stop_details"]
415-
if msg.get("usage"):
416-
response["usage"].update(msg["usage"])
475+
response["usage"].update(_sse_dict(msg.get("usage")))
417476

418477
elif event_type == "content_block_start":
419-
block = data.get("content_block", {})
420-
block_index = data.get("index", len(response["content"]))
478+
block = _sse_dict(data.get("content_block"))
479+
block_index = _sse_index(data.get("index"), len(response["content"]))
421480
btype = block.get("type")
422481
current_block = {
423482
"type": btype,
424483
"index": block_index,
425484
}
426485
if btype == "text":
427-
current_block["text"] = block.get("text", "")
486+
current_block["text"] = _sse_str(block.get("text"))
428487
elif btype == "tool_use":
429488
current_block["id"] = block.get("id")
430489
current_block["name"] = block.get("name")
@@ -433,7 +492,7 @@ def _parse_sse_to_response(self, sse_data: str, provider: str) -> dict[str, Any]
433492
# Thinking block — accumulate text via
434493
# `thinking_delta`; signature arrives via
435494
# `signature_delta` (single value, not accumulated).
436-
current_block["thinking_buffer"] = block.get("thinking", "")
495+
current_block["thinking_buffer"] = _sse_str(block.get("thinking"))
437496
if "signature" in block:
438497
current_block["signature"] = block["signature"]
439498
elif btype == "redacted_thinking":
@@ -450,31 +509,33 @@ def _parse_sse_to_response(self, sse_data: str, provider: str) -> dict[str, Any]
450509
# {type, index}. Mirrors the sibling reconstructor
451510
# `_reconstruct_anthropic_response`, which does `dict(block)`.
452511
for _k, _v in block.items():
453-
if _k != "type":
512+
if _k not in _BLOCK_SCRATCH_KEYS:
454513
current_block[_k] = _v
455514
blocks_by_index[block_index] = current_block
456515

457516
elif event_type == "content_block_delta":
458517
# Resolve the target block by index (preferred) or fall
459518
# back to current_block for legacy linear streams.
460-
idx = data.get("index")
519+
idx = _sse_index(data.get("index"))
461520
target = (blocks_by_index.get(idx) if idx is not None else None) or current_block
462521
if target is not None:
463-
delta = data.get("delta", {})
522+
# `"delta": null` bypasses a `.get(..., {})` default the same
523+
# way `"message": null` does on message_start.
524+
delta = _sse_dict(data.get("delta"))
464525
dtype = delta.get("type")
465526
if dtype == "text_delta":
466-
target["text"] = target.get("text", "") + delta.get("text", "")
527+
target["text"] = _sse_str(target.get("text")) + _sse_str(delta.get("text"))
467528
elif dtype == "input_json_delta":
468529
# Accumulate partial JSON for tool input.
469-
partial = delta.get("partial_json", "")
470-
target["_partial_json"] = target.get("_partial_json", "") + partial
530+
partial = _sse_str(delta.get("partial_json"))
531+
target["_partial_json"] = _sse_str(target.get("_partial_json")) + partial
471532
elif dtype == "thinking_delta":
472533
# Accumulate thinking text into the dedicated
473534
# buffer so it never collides with `text` on
474535
# text blocks (separate field per guide §2.7).
475-
target["thinking_buffer"] = target.get("thinking_buffer", "") + delta.get(
476-
"thinking", ""
477-
)
536+
target["thinking_buffer"] = _sse_str(
537+
target.get("thinking_buffer")
538+
) + _sse_str(delta.get("thinking"))
478539
elif dtype == "signature_delta":
479540
# Single value, not accumulated. Last-write
480541
# wins per Anthropic spec.
@@ -485,13 +546,18 @@ def _parse_sse_to_response(self, sse_data: str, provider: str) -> dict[str, Any]
485546
# list so multi-citation blocks reconstruct
486547
# correctly. Per guide §2.5: each delta carries
487548
# one full citation object under `citation`.
488-
citations = target.setdefault("citations", [])
549+
# A non-standard block start can copy a non-list
550+
# `citations` through, which `.append` would reject.
551+
citations = target.get("citations")
552+
if not isinstance(citations, list):
553+
citations = []
554+
target["citations"] = citations
489555
citation = delta.get("citation")
490556
if citation is not None:
491557
citations.append(citation)
492558

493559
elif event_type == "content_block_stop":
494-
idx = data.get("index")
560+
idx = _sse_index(data.get("index"))
495561
target = (blocks_by_index.get(idx) if idx is not None else None) or current_block
496562
if target is not None:
497563
# Parse accumulated JSON into `input` for any block that
@@ -525,13 +591,15 @@ def _parse_sse_to_response(self, sse_data: str, provider: str) -> dict[str, Any]
525591
current_block = None
526592

527593
elif event_type == "message_delta":
528-
delta = data.get("delta", {})
594+
delta = _sse_dict(data.get("delta"))
529595
if "stop_reason" in delta:
530596
response["stop_reason"] = delta["stop_reason"]
531597
if "stop_details" in delta:
532598
response["stop_details"] = delta["stop_details"]
533-
if data.get("usage"):
534-
response["usage"].update(data["usage"])
599+
# Type-checked, not truthiness-checked: `dict.update` on a
600+
# non-mapping raises. The message_start twin above already
601+
# guarded this; this sibling did not.
602+
response["usage"].update(_sse_dict(data.get("usage")))
535603

536604
return response if response.get("content") else None
537605

0 commit comments

Comments
 (0)