Support X-Forwarded-Host in ProxyHeadersMiddleware#2922
Conversation
|
📖 Docs preview: https://1adee858.uvicorn.pages.dev |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f68cc4f524
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| x_forwarded_host = headers[b"x-forwarded-host"].decode("latin1").strip() | ||
|
|
||
| if x_forwarded_host: | ||
| host, port = _parse_host_port(x_forwarded_host) | ||
| if not port: |
There was a problem hiding this comment.
Parse only one X-Forwarded-Host hop before rewriting host
When a trusted proxy sends a multi-hop X-Forwarded-Host value (comma-separated, which is common with chained proxies), this code parses the entire header as one host instead of selecting a single hop first. That means scope["server"] can be set to an invalid hostname like "example.com,proxy.local" (or lose a valid port in IPv6+port chains), and the rewritten Host header will also contain the full list, which can break URL generation/redirect behavior in proxied production setups. Split the header into hops (and choose the intended trusted hop) before calling _parse_host_port.
Useful? React with 👍 / 👎.
Closes #965
Summary
Adds
X-Forwarded-Hostsupport toProxyHeadersMiddleware. When the connecting peer is inforwarded_allow_ipsand the header is present, the middleware:hostrequest header inscope["headers"]with the forwarded valuescope["server"]to(host, port), parsing IPv4/IPv6/host:portvia the existing_parse_host_porthelper used forX-Forwarded-For443when the (possibly already-rewritten)scope["scheme"]ishttpsorwss, and80otherwiseThis matches the behavior of Werkzeug's
ProxyFix(linked by @Kludex in the issue as the reference) and addresses theredirect_slashesandStaticFilesredirect breakage that surfaces when a reverse proxy rewritesHostto its internal hostname (e.g., Caddy 2.11+, Heroku, many CDNs).Behaviour
forwarded_allow_ipscheck; no new CLI flag.X-Forwarded-Hostis a no-op.proto -> for -> host, so the host block can use the forwarded scheme to pick the default port.Hostheader is replaced (not appended) - matches Werkzeug and Hypercorn.Tests
New cases cover untrusted-peer ignore, hostname /
host:port/ IPv4 / IPv4+port / bracketed IPv6 / IPv6+port shapes, scheme-driven default ports for http/https/ws/wss, host-header replacement (no duplicates), and the combined for+proto+host case.Checklist
AI Disclaimer
This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.