fix(http): read framing from every header value, not just the first

Adversarially probing the re-gated head found the same poisoning reachable by
DUPLICATING a framing header instead of omitting one. `Message.get()` returns
only the first occurrence, so `Content-Length: 0` sent ahead of the real length
made the body invisible to every reader: nothing was drained and the real bytes
were parsed as the next request line. Reproduced live on the otherwise-fixed
head, over one pooled socket:

    HTTP/1.1 403 Forbidden                    <- sidecar provenance reject
    HTTP/1.1 400 Bad request syntax ('{"stale": true}GET /api/health/agent ...')

    HTTP/1.1 400 Bad Request                  <- /api/upload reject
    HTTP/1.1 400 Bad request syntax ('--x')

api/helpers.py now reads framing through `_framing_header_values()`, which
returns every value the wire carried. `request_declares_body()` treats
disagreeing or unparseable lengths as a declared body (they cannot prove the
body's absence), and `unsupported_transfer_encoding()` inspects every declared
coding so `identity` cannot mask a `chunked` behind it. Two agreeing zeroes are
still no body -- repetition alone is not a conflict.

New `unreadable_content_length()` gates the multipart preflight: with no honest
byte count there is nothing to drain, so all four handlers refuse with 400
"Invalid Content-Length" before touching rfile, connection armed for close.

13 regressions, each verified red without this commit: the helper predicates on
a real email.message.Message, the production sidecar 403, all four multipart
handlers, and five pipelined real-socket tests proving the follow-up request is
never answered off a poisoned socket.
This commit is contained in:
Rodrigo Gomes da Silva authored and n committed 2026-09-22 12:06:09 +00:00
1 parent b3a031a19d
commit fb7664bc40
4 files changed
+223 -32

No files matched your search

@@ -212,6 +212,89 @@ def test_request_declares_body_tolerates_a_handler_without_headers():
assert request_declares_body(cast(Any, SimpleNamespace(headers=object()))) is False
# ── A duplicated framing header hides the body from a first-value-only read ────
#
# `Message.get()` returns only the FIRST occurrence, so `Content-Length: 0`
# ahead of the real length reads back as an empty body: nothing is drained and
# the real bytes are parsed as the next request line. Verified live against the
# running server before the fix -- 403 then
# `400 Bad request syntax ('{"stale": true}GET /api/health/agent HTTP/1.1')`,
# and on /api/upload a 400 then `400 Bad request syntax ('--x')`.
def _message_with(raw_headers: str):
"""Build the email.message.Message a production handler really carries."""
from email.parser import Parser
return SimpleNamespace(headers=Parser().parsestr(raw_headers + "\r\n"))
def test_duplicate_conflicting_content_length_declares_a_body():
from api.helpers import request_declares_body
handler = _message_with("Host: x\r\nContent-Length: 0\r\nContent-Length: 15\r\n")
assert request_declares_body(cast(Any, handler)) is True
def test_duplicate_agreeing_zero_content_length_declares_no_body():
"""Repetition alone is not a conflict -- two honest zeroes still frame nothing."""
from api.helpers import request_declares_body
handler = _message_with("Host: x\r\nContent-Length: 0\r\nContent-Length: 0\r\n")
assert request_declares_body(cast(Any, handler)) is False
def test_repeated_transfer_encoding_is_detected_beyond_the_first_value():
"""`identity` first must not mask a chunked coding behind it."""
from api.helpers import unsupported_transfer_encoding
handler = _message_with(
"Host: x\r\nTransfer-Encoding: identity\r\nTransfer-Encoding: chunked\r\n"
)
assert unsupported_transfer_encoding(cast(Any, handler)) == "chunked"
def test_unreadable_content_length_flags_conflict_and_garbage_only():
from api.helpers import unreadable_content_length
assert unreadable_content_length(cast(Any, _message_with("Content-Length: 5\r\n"))) is False
assert unreadable_content_length(cast(Any, _message_with("Host: x\r\n"))) is False
assert unreadable_content_length(
cast(Any, _message_with("Content-Length: 5\r\nContent-Length: 5\r\n"))
) is False
assert unreadable_content_length(
cast(Any, _message_with("Content-Length: 0\r\nContent-Length: 9\r\n"))
) is True
assert unreadable_content_length(cast(Any, _message_with("Content-Length: nope\r\n"))) is True
def test_sidecar_get_with_duplicate_content_length_closes_connection(monkeypatch):
"""The conflicting-framing GET reaches the production provenance rejection."""
import api.routes as routes
from email.parser import Parser
headers = Parser().parsestr("Host: x\r\nContent-Length: 0\r\nContent-Length: 15\r\n\r\n")
handler = SimpleNamespace(
path="/api/extensions/ext1/sidecar/proxy",
command="GET",
headers=headers,
close_connection=False,
)
monkeypatch.setattr(routes, "_match_extension_sidecar_proxy_path", lambda _path: ("ext1", "/proxy"))
monkeypatch.setattr(routes, "_check_same_origin_browser_request", lambda _handler, **_: False)
monkeypatch.setattr(routes, "j", lambda _handler, _payload, status=200: None)
routes._handle_extension_sidecar_proxy(
cast(Any, handler), SimpleNamespace(path=handler.path, query=""), "GET"
)
assert handler.close_connection is True
def test_csp_report_rate_limited_closes_connection(monkeypatch):
"""A rate-limited CSP report is dropped before its body is read."""
import api.routes as routes
@@ -335,6 +418,28 @@ def test_non_numeric_content_length_still_closes(handler_name, monkeypatch):
assert handler.close_connection is True
@pytest.mark.parametrize("handler_name", _MULTIPART_HANDLERS)
def test_duplicate_content_length_multipart_rejects_before_read(handler_name, monkeypatch):
"""Disagreeing lengths give no honest byte count -- refuse without reading.
Reading the first value (0) made the body invisible: the handler answered
"No file field in request" with keep-alive and left the real multipart bytes
on the socket, where the server parsed them as the next request line.
"""
from email.parser import Parser
headers = Parser().parsestr(
"Content-Type: multipart/form-data; boundary=x\r\n"
"Content-Length: 0\r\n"
"Content-Length: 88\r\n\r\n"
)
handler, answered = _run_multipart_handler(monkeypatch, handler_name, headers, _NeverRead())
assert answered["status"] == 400, answered
assert answered["payload"]["error"] == "Invalid Content-Length", answered
assert handler.close_connection is True
# ── Header serialization: exactly one Connection: close, never a crash ────────
#
# The advertisement lives in Handler.end_headers() via
@@ -157,3 +157,40 @@ def test_sidecar_get_with_a_body_closes_and_cannot_poison_the_socket():
)
_assert_single_closed_response(answered, b"403", _BODY)
# ── Framing hidden behind a DUPLICATED header ─────────────────────────────────
#
# `Content-Length: 0` sent ahead of the real length: `Message.get()` returns only
# the first value, so every reader saw an empty body and drained nothing. Both
# cases below were reproduced live on the otherwise-fixed head, poisoning the
# socket exactly as the single-header cases used to:
# sidecar GET -> 403, then 400 Bad request syntax ('{"stale": true}GET /...')
# /api/upload -> 400, then 400 Bad request syntax ('--x')
def test_duplicate_content_length_sidecar_get_closes_and_cannot_poison_the_socket():
answered = _pipelined_after(
"GET /api/extensions/probe/sidecar/ping HTTP/1.1\r\n"
"Host: 127.0.0.1\r\n"
"Content-Type: application/json\r\n"
"Content-Length: 0\r\n"
f"Content-Length: {len(_BODY)}\r\n"
"\r\n".encode() + _BODY
)
_assert_single_closed_response(answered, b"403", _BODY)
@pytest.mark.parametrize("path", _MULTIPART_UPLOAD_PATHS)
def test_duplicate_content_length_upload_closes_and_cannot_poison_the_socket(path):
answered = _pipelined_after(
f"POST {path} HTTP/1.1\r\n"
"Host: 127.0.0.1\r\n"
"Content-Type: multipart/form-data; boundary=x\r\n"
"Content-Length: 0\r\n"
f"Content-Length: {len(_MULTIPART_PAYLOAD)}\r\n"
"\r\n".encode() + _MULTIPART_PAYLOAD
)
_assert_single_closed_response(answered, b"400", _MULTIPART_PAYLOAD)