fix: combine repeated request headers instead of keeping the last

YB
1d ago
+51-4

Refs #7521. This is part 1 of the split proposed on the issue: steps 1 and 2 of the fix. It leaves raw_headers multi-value access (getlist, step 3) to a follow-up PR.

_decode_asgi_headers built a dict from the ASGI header pairs, so a repeated field kept only its last value. ASGI keeps repeats as separate pairs. The function now combines them in order:

  • repeated cookie fields are joined with "; " (RFC 9113 §8.2.3);
  • any other repeated field is joined with ", " (RFC 9110 §5.3).

It still returns the same dict[str, str], so RouteVar.HEADERS and everything that reads it keep their shape. With the combined values:

  • router.headers.cookie has every cookie;
  • router.session.client_ip comes from the first x-forwarded-for hop, because the existing .partition(",")[0] now sees first, second;
  • raw_headers[name] shows the combined value.

The credential filtering from #7360 works on the joined dict, so a repeated cookie is still dropped entirely from what reaches the frontend.

Tests

  • test_decode_asgi_headers: the old "later duplicates win" assertion now expects a=1; b=2. A new case interleaves two x-forwarded-for fields with another header and expects 10.0.0.1, 10.0.0.2, 10.0.0.3.
  • test_static_router_data_keeps_every_repeated_header_field (new) goes through EventNamespace._build_static_router_data with two cookie and two x-forwarded-for fields. It checks the joined headers, that CLIENT_IP is the first hop (203.0.113.7, not the proxy), and that HeaderData.from_router_data(...).cookie keeps both cookies.

Both fail on main with the issue's symptom ({'cookie': 'b=2'}, 'other=2'). With the change, all of tests/units/test_app.py passes (194).

I also ran uv run pytest tests/units --cov --no-cov-on-fail --cov-report=: 12014 passed, 26 skipped, total coverage 80.03%. ruff check, ruff format --check and pyright on the two changed files are clean (0 errors).

Performance

The function now runs a loop instead of a comprehension. I measured that with timeit on the benchmark's unique-header shape on macOS arm64. It's 1.10–1.14× slower: 0.27 vs 0.25 µs for 4 headers, 4.08 vs 3.57 µs for 64. It is only called from _build_static_router_data, once per connection, not per event.

Not in this PR

Step 3: keeping every field in raw_headers for getlist() and multi_items(). The open questions for it are on the issue.