fix: combine repeated request headers instead of keeping the last

[ybai08]

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.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

The PR preserves repeated request headers instead of keeping only the last value.

• Request headers keep every repeated value in the router data.

Acknowledged scope: ybai08 explicitly deferred raw_headers.getlist() and multi_items() to a follow-up PR.

_{Reviews (1) · Last reviewed commit: "fix: combine repeated request headers in..." · Reviewed by Greptile}

[cubic-dev-ai]

2 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="reflex/app.py">

<violation number="1" location="reflex/app.py:2110">
P2: `name` is used case-sensitively even though ASGI does not require servers to lowercase header names. Normalize it before aggregation; otherwise repeated `Cookie` values use `", "` and mixed-case `X-Forwarded-For` is ignored.</violation>

<violation number="2" location="reflex/app.py:2113">
P2: Joining repeated `x-forwarded-for` lines in order changes which hop `CLIENT_IP` uses. Before, the last line won, so a proxy that adds its own `X-Forwarded-For` line (for example nginx `proxy_add_header`) overrode a client-sent one. Now the client-supplied first line becomes `CLIENT_IP`, so a client can spoof it. Confirm this trust change is intended, or keep proxy-added XFF lines authoritative.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Joining repeated x-forwarded-for lines in order changes which hop CLIENT_IP uses. Before, the last line won, so a proxy that adds its own X-Forwarded-For line (for example nginx proxy_add_header) overrode a client-sent one. Now the client-supplied first line becomes CLIENT_IP, so a client can spoof it. Confirm this trust change is intended, or keep proxy-added XFF lines authoritative.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At reflex/app.py, line 2113:

<comment>Joining repeated `x-forwarded-for` lines in order changes which hop `CLIENT_IP` uses. Before, the last line won, so a proxy that adds its own `X-Forwarded-For` line (for example nginx `proxy_add_header`) overrode a client-sent one. Now the client-supplied first line becomes `CLIENT_IP`, so a client can spoof it. Confirm this trust change is intended, or keep proxy-added XFF lines authoritative.</comment>

<file context>
@@ -2095,13 +2095,25 @@ def _sio_loads(data: str | bytes, **kwargs: Any) -> Any:
+        name = raw_name.decode("utf-8")
+        value = raw_value.decode("utf-8")
+        if name in decoded:
+            decoded[name] += ("; " if name == "cookie" else ", ") + value
+        else:
+            decoded[name] = value
</file context>

[cubic-dev-ai]

P2: name is used case-sensitively even though ASGI does not require servers to lowercase header names. Normalize it before aggregation; otherwise repeated Cookie values use ", " and mixed-case X-Forwarded-For is ignored.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At reflex/app.py, line 2110:

<comment>`name` is used case-sensitively even though ASGI does not require servers to lowercase header names. Normalize it before aggregation; otherwise repeated `Cookie` values use `", "` and mixed-case `X-Forwarded-For` is ignored.</comment>

<file context>
@@ -2095,13 +2095,25 @@ def _sio_loads(data: str | bytes, **kwargs: Any) -> Any:
-    return {k.decode("utf-8"): v.decode("utf-8") for (k, v) in headers}
+    decoded: dict[str, str] = {}
+    for raw_name, raw_value in headers:
+        name = raw_name.decode("utf-8")
+        value = raw_value.decode("utf-8")
+        if name in decoded:
</file context>

Suggested change

	name = raw_name.decode("utf-8")
	name = raw_name.decode("utf-8").lower()