Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

X-Forwarded-Host doesn't support setting a port #2191

Open
ignasi35 opened this issue Sep 4, 2018 · 6 comments
Open

X-Forwarded-Host doesn't support setting a port #2191

ignasi35 opened this issue Sep 4, 2018 · 6 comments
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:core Issues related to the akka-http-core module t:model Issues around the model classes and its functionality

Comments

@ignasi35
Copy link
Contributor

ignasi35 commented Sep 4, 2018

According to the Forwarded spec:

The syntax for a "host" value, after potential quoted-string
unescaping, MUST conform to the Host ABNF described in Section 5.4 of
[RFC7230].

Then, Mozilla's MDN documents the X-Forwarded-Host syntax ambiguously. First is states:

The X-Forwarded-Host (XFH) header is a de-facto standard header for identifying the original host requested by the client in the Host HTTP request header.

which seems to imply both Host and X-Forwarded-Host use the same syntax (as specified in Forwarded). But then on the Syntax section it's specified as:

X-Forwarded-Host: <host>

which is different from the MDN spec for Host:

Host: <host>:<port>

The current implementation in Akka-HTTP uses the <host> syntax and cites the MDN page linked above (which can be interpreted in different ways).

X-Forwarded-Host is a de facto standard so we could be debating forever. Per the Forwarded spec, I think X-Forwarded-Host should allow setting a port.

@ignasi35
Copy link
Contributor Author

ignasi35 commented Sep 4, 2018

An example use case:

  • In Lagom's dev mode, the Service Gateway (aka Reverse Proxy) listens on port :9000 and redirects requests to downstream services listening on random ports. Currently the processed request receive X-Forwarded-Host: localhost instead of X-Forwarded-Host: localhost:9000.

@jrudolph
Copy link
Contributor

jrudolph commented Sep 4, 2018

I agree that could be useful. The header class is currently still marked @ApiMayChange so we may be able to just do the change.

(I don't quite understand the need for that header, why would a reverse proxy rewrite the Host header in the first place and not just forward it to the backend server unchanged?)

@jrudolph jrudolph added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:model Issues around the model classes and its functionality t:core Issues related to the akka-http-core module labels Sep 4, 2018
@TimMoore
Copy link

TimMoore commented Sep 5, 2018

@jrudolph this is discussed by the HTTP spec in RFC 7230

When a proxy receives a request with an absolute-form of
request-target, the proxy MUST ignore the received Host header field
(if any) and instead replace it with the host information of the
request-target. A proxy that forwards such a request MUST generate a
new Host field-value based on the received request-target rather than
forward the received Host field-value.

But I think in this spec, "proxy" always refers to a forward client-side proxy, so it's a bit ambiguous about the case of a reverse proxy. It looks like implementations vary in practice, with some preserving the original Host and others replacing it.

I'm not sure where the X-Forwarded-Host convention originated, but it looks like some implementations use a separate X-Forwarded-Port header (as well as an X-Forwarded-Scheme or X-Forwarded-Proto to distinguish HTTP vs. HTTPS).

Anyway, it seems like maybe for a non-standard header, it should be lax about what's allowed as the value.

@jlprat
Copy link
Contributor

jlprat commented Sep 6, 2018

The way I understood those X-Forwarded-* headers is the following:
X-Forwarded-Host: contains only the host
X-Forwarded-Port: contains only the port

My interpretation is, Forwarded header is complicated to parse, this is why this set of non standard headers were introduced, to separate each component on a different header.

@ignasi35
Copy link
Contributor Author

The way I understood those X-Forwarded-* headers is the following:
X-Forwarded-Host: contains only the host
X-Forwarded-Port: contains only the port

The problem with de-facto headers is that you are free to pick and choose. MDN doesn't acknowledge the existence of a X-Forwarded-Port header and Wikipedia uses a host:port syntax in the X-Forwarded-Host example. But then Heroku uses X-F-Port and not X-F-Host. 🤷‍♂️

I think @TimMoore's suggestion is the best path forward:

Anyway, it seems like maybe for a non-standard header, it should be lax about what's allowed as the value.

@jlprat
Copy link
Contributor

jlprat commented Sep 14, 2018

Yes, I think we could be lax with what's in that header

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:core Issues related to the akka-http-core module t:model Issues around the model classes and its functionality
Projects
None yet
Development

No branches or pull requests

4 participants