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

Jetty 12 generates wrong Host header #10306

Closed
ghost opened this issue Aug 13, 2023 · 6 comments · Fixed by #10311
Closed

Jetty 12 generates wrong Host header #10306

ghost opened this issue Aug 13, 2023 · 6 comments · Fixed by #10311
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@ghost
Copy link

ghost commented Aug 13, 2023

Jetty version: 12.0.0

Jetty environment: ee10 (embedded Jetty server configured to use HTTP/2)

Java version: openjdk version "20.0.1" 2023-04-18
OpenJDK Runtime Environment (Red_Hat-20.0.1.0.9-2.rolling.fc38) (build 20.0.1+9)
OpenJDK 64-Bit Server VM (Red_Hat-20.0.1.0.9-2.rolling.fc38) (build 20.0.1+9, mixed mode, sharing)

OS: Fedora 38, Ubuntu 23.04

Jetty returns the wrong host header when a request is forwarded from a different port. In my case the client sends a request to the default port 443 which is then forwarded to port 8443 which is the port Jetty listens on. The host header then incorrectly contains the port number 8443.

Server receives this (pardon the brackets):

request.getRequestURL().toString():

URL=[https://demo.jambo.software/demo.jambo.software/s;x=13y762q065p53vl8nbmb4uf720]

request.getHeader(...):

HEADER[sec-fetch-mode]=[cors]
HEADER[content-length]=[26]
HEADER[sec-fetch-site]=[same-origin]
HEADER[accept-language]=[en-US,en;q=0.9]
HEADER[cookie]=[13y762q065p53vl8nbmb4uf720=A25F3449-F86C-4545-805E-45F947465397]
HEADER[origin]=[https://demo.jambo.software/]
HEADER[Host]=[demo.jambo.software:8443]
HEADER[accept]=[/]
HEADER[sec-gpc]=[1]
HEADER[sec-ch-ua]=["Not/A)Brand";v="99", "Brave";v="115", "Chromium";v="115"]
HEADER[sec-ch-ua-mobile]=[?0]
HEADER[sec-ch-ua-platform]=["Linux"]
HEADER[content-type]=[application/vnd.piglet]
HEADER[accept-encoding]=[gzip, deflate, br]
HEADER[user-agent]=[Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/115.0.0.0 Safari/537.36]
HEADER[sec-fetch-dest]=[empty]

While the netword tab of the browser gives this for the request:

Request URL:https://demo.jambo.software/demo.jambo.software/s;x=13y762q065p53vl8nbmb4uf720
Request Method:POST
Status Code:200
Remote Address:136.144.238.65:443
Referrer Policy:no-referrer

:authority:demo.jambo.software
:method:POST
:path:/demo.jambo.software/s;x=13y762q065p53vl8nbmb4uf720
:scheme:https
Accept:/
Accept-Encoding:gzip, deflate, br
Accept-Language:en-US,en;q=0.9
Content-Length:26
Content-Type:application/vnd.piglet
Cookie:13y762q065p53vl8nbmb4uf720=A25F3449-F86C-4545-805E-45F947465397
Origin:https://demo.jambo.software/
Sec-Ch-Ua:"Not/A)Brand";v="99", "Brave";v="115", "Chromium";v="115"
Sec-Ch-Ua-Mobile:?0
Sec-Ch-Ua-Platform:"Linux"
Sec-Fetch-Dest:empty
Sec-Fetch-Mode:cors
Sec-Fetch-Site:same-origin
Sec-Gpc:1
User-Agent:Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/115.0.0.0 Safari/537.36

@ghost ghost added the Bug For general bugs on Jetty side label Aug 13, 2023
@gregw
Copy link
Contributor

gregw commented Aug 14, 2023

@joakime, I recall you and I working on the HostPort code.... which I think we have got wrong here:

https://github.com/eclipse/jetty.project/blob/52d94174e2c7a6e794c6377dcf9cd3ed0b9e1806/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java#L426-L450

If the URI (which includes the authority received either via the Host header or the HTTP2 authority header) does not have a port, then the server port should be the logical port for the scheme (80 or 443), unless it has been forced to a specific port by the server authority.
However, we are looking for the local port before considering the default port. Can you remember why we (and by we I mean I) did that?

@joakime
Copy link
Contributor

joakime commented Aug 14, 2023

@gregw I think it came about when added support to override the getLocalName() and getLocalPort() features of the Request object in PR #7357

@uschindler
Copy link

uschindler commented Aug 14, 2023

Hi,
it looks like #10305 is caused by the same bug. The ForwardedRequestCustomizer does not respect the default host header (I misread the code). But here the port number gets added. In my case the original NGINX passed the Host header from the client unmodified, causing havoc. If I use the "X-Forwarded_Host" header in NGINX, the behaviour is correct.

So this looks like a duplicate of #10304 !

@uschindler
Copy link

It also happens with HTTP/1.1.

gregw added a commit that referenced this issue Aug 14, 2023
Fix #10306 getServerHost
@gregw gregw linked a pull request Aug 14, 2023 that will close this issue
@gregw
Copy link
Contributor

gregw commented Aug 14, 2023

I've done a tentative fix in #10311. It initially has not broken any tests that I tried (doing a full build now), so that is a) good in that nothing depending on previous behaviour ; b) bad we are not testing this very well!
So at the least, we will need some more tests to check this is the problem and that it is fix. Meanwhile, if you want to test the branch built by CI, that would be useful.

gregw added a commit that referenced this issue Aug 15, 2023
work in progress
gregw added a commit that referenced this issue Aug 15, 2023
more javadoc
gregw added a commit that referenced this issue Aug 18, 2023
@gregw
Copy link
Contributor

gregw commented Aug 18, 2023

Should be fixed with #10311

@gregw gregw closed this as completed Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants