Skip to content

fix(middleware): restore host-port stripping in AllowedHostsMiddleware#4922

Closed
chuenchen309 wants to merge 1 commit into
litestar-org:mainfrom
chuenchen309:fix/allowed-hosts-port-stripping
Closed

fix(middleware): restore host-port stripping in AllowedHostsMiddleware#4922
chuenchen309 wants to merge 1 commit into
litestar-org:mainfrom
chuenchen309:fix/allowed-hosts-port-stripping

Conversation

@chuenchen309

@chuenchen309 chuenchen309 commented Jul 14, 2026

Copy link
Copy Markdown

What

A request whose Host header includes a port (e.g. example.com:8080) is incorrectly rejected with 400 by AllowedHostsMiddleware, even when the bare hostname (example.com) is present in allowed_hosts.

Root cause

Commit c418961 ("fix: fix typing after merge") changed:

if host := headers.get("host").split(":")[0]:

into:

if (host := headers.get("host")) is not None and host.split(":")[0]:

The walrus assignment now binds host to the full header value (including any port), and host.split(":")[0] is only used for a truthiness check — its result is discarded. So self.allowed_hosts_regex.fullmatch(host) downstream still receives the port-suffixed value, which never matches a bare-hostname regex pattern.

Fix

Rebind host via a second walrus assignment so the stripped (port-free) value is what's actually used for matching:

if (host := headers.get("host")) is not None and (host := host.split(":")[0]):

How I verified this

  • Traced the regression to c418961 via git log -p --follow -- litestar/middleware/allowed_hosts.py.
  • Reproduced standalone with a minimal ASGI script directly invoking AllowedHostsMiddleware.
  • Added a parametrize case to the existing test_middleware_allowed_hosts test (("http://x.example.com:8080", None, HTTP_200_OK)); confirmed it fails on the current code (assert 400 == 200) before the fix.
  • Applied the one-line fix, confirmed all 14 tests in test_allowed_hosts_middleware.py pass.
  • Ran the full tests/unit/test_middleware/ suite: 776 passed, no regressions.
  • mypy litestar/middleware/allowed_hosts.py — clean.
  • pre-commit run on changed files — clean (ruff check/format, unasyncd, typos).

Disclosure

I used AI assistance (Claude) to help investigate and draft this fix, but I personally reviewed the root-cause trace, ran the repro and full test suite myself, and reviewed the final diff before submitting. Happy to answer any questions about the change.


📚 Documentation preview 📚: https://litestar-org.github.io/litestar-docs-preview/4922

Requests with a Host header that includes a port (e.g. `example.com:8080`)
were incorrectly rejected with 400, even when the bare hostname was in
`allowed_hosts`. Commit c418961 ("fix: fix typing after merge") changed
the walrus assignment to only use `host.split(":")[0]` for a truthiness
check, without rebinding `host` to the stripped value — so the full
header (with port) was passed to `allowed_hosts_regex.fullmatch()`,
which never matches a bare-hostname pattern.

Fix: rebind `host` via a second walrus assignment so the stripped value
is what's actually used downstream.

How verified: traced the regression to c418961 via `git log -p --follow`
on this file, reproduced with a standalone ASGI repro script, added a
parametrize case to the existing test_middleware_allowed_hosts test
(confirmed it fails with the old code: assert 400 == 200), applied the
one-line fix, confirmed all 14 tests in this file pass, then ran the full
unit/test_middleware/ suite (776 passed, no regressions). mypy clean,
pre-commit clean (ruff, unasyncd, typos).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chuenchen309
chuenchen309 requested review from a team as code owners July 14, 2026 09:45
@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.29%. Comparing base (bd18fd4) to head (236213f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4922   +/-   ##
=======================================
  Coverage   67.29%   67.29%           
=======================================
  Files         293      293           
  Lines       15228    15228           
  Branches     1728     1728           
=======================================
  Hits        10248    10248           
  Misses       4833     4833           
  Partials      147      147           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@chuenchen309

Copy link
Copy Markdown
Author

Thanks for the review time on this — I found while doing some further digging that #4873 already fixes this exact regression (referencing #4869), opened back on 2026-06-20, well before my independent discovery of it today. Since it's the same root cause and fix, I'll close this one in favor of the earlier PR rather than duplicate the review effort. Apologies for not catching the existing PR before submitting — will be more careful to cross-check open PRs against the exact file path before starting work in the future.

@provinzkraut

Copy link
Copy Markdown
Member

Thanks for the review time on this

There was no review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants