TrustedHostMiddleware ignores port in Host header

[blipk]

Cross-post from an issue at fastapi/fastapi#5818 as I didn't realise it was starlette providing this underneath.

app = FastAPI()
app.add_middleware(
    TrustedHostMiddleware,
    allowed_hosts=["localhost:8000"]
)

Using the provided example code and making a request from localhost:8000 I get an "Invalid host header" response, even though looking at dev tools in chrome the header is Host: localhost:8000

If I change to allowed_hosts=["localhost"] it works fine.

I understand it's a trusted host, and not a trusted port, but seems strange to me that it says invalid host header even though thats exactly what the browser is sending in the host header.

Also see comment by @jgould22

The relevant bit of code is here https://github.com/encode/starlette/blob/master/starlette/middleware/trustedhost.py#L38

I agree since the Host header can include port (https://www.rfc-editor.org/rfc/rfc9110.html#field.host) it should not be trimmed off before the check.

Even the docs refer only to a "correctly set Host header" https://www.starlette.io/middleware/#trustedhostmiddleware .

[blipk]

Hi @iudeen

Why was this converted to a discussion? It's an actual issue.

[iudeen]

The starting point for issues should usually be a discussion...

https://github.com/encode/starlette/discussions

Possible bugs may be raised as a "Potential Issue" discussion, feature requests may be raised as an "Ideas" discussion. We can then determine if the discussion needs to be escalated into an "Issue" or not.

This will help us ensure that the "Issues" list properly reflects ongoing or needed work on the project.

Discussions are monitored and addressed by maintainers in the repo with equal importance.

[Kludex]

Any reference on what other web frameworks do in this case?

[ramnes]

As to Flask, it doesn't give anything to restrict against a list of hosts, as far as I can remember / tell.

[jgould22]

Flask sits on top of Werkzeug (iirc) which provides this method/behavior though I am not sure it is used/exposed in flask itself.

https://werkzeug.palletsprojects.com/en/2.2.x/wsgi/#werkzeug.wsgi.get_host

[ramnes]

And it removes the port as well: https://github.com/pallets/werkzeug/blob/6e8bc96f289d0d1a75817469f37eb29ca80393a5/src/werkzeug/sansio/utils.py#L24-L28

[iudeen]

In my experience, ports are not considered in hosts. And seeing references from other frameworks, I don't think this is an issue that needs fix🤔

May be we can raise a warning and coerce the user input by removing port.

[blipk]

@iudeen
The point is it's a trusted HostHeader, not just Host, and the HostHeader sometimes includes the port as per the RFC.
And Host isn't necessarily just just host name, it's also the host port.

Fair enough it's not hugely important, but as per my original example which is quite common and the default port, you could easily be confused and there's a case for why it should work based on the RFC and used nomenclature + starlette docs.

Seeing as it's not a huge issue, and I couldn't find an issue elsewhere bringing it up, I thought I'd at least put it out there for people to be aware.

If you guys are happy to stick with the non-spec implementation that seems to be the standard in related frameworks, then that's fine.

Perhaps updating the documentation would be the easiest solution, rather than shimming warnings into the codebase.

[ramnes]

Is there any use case for actually checking the port?

If not, this is probably just a DX issue; we could check that allowed hosts don't contain ports and either raise an exception, or remove the ports and log a warning.

[blipk]

The docs explicitly say it's checking the Host header though, so it doesn't make much sense right now.

Why not add the option to check ports or not?

[iudeen]

I stumbled upon a discussion here, where suggestion is similar to the one you are suggesting. @ramnes