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.

Sign up for GitHub

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

Jump to bottom

Request without Host header fails with NullPointerException in ForwardedRequestCustomizer #5443

Closed
oyvindhorneland opened this issue Oct 13, 2020 · 4 comments · Fixed by #5445
Closed

Request without Host header fails with NullPointerException in ForwardedRequestCustomizer #5443

oyvindhorneland opened this issue Oct 13, 2020 · 4 comments · Fixed by #5445
Assignees
@joakime
Labels
Bug For general bugs on Jetty side
Projects
Jetty 9.4.33

Comments

@oyvindhorneland
Copy link

Jetty version
9.4.32.v20200930

Java version
11

OS type/version
Red Hat Enterprise Linux Server 7

Description
Requests (Healthcheck probes) using HTTP/1.0 without Host header started to fail after upgrade from 9.4.31.v20200723 to 9.4.32.v20200930 with http-forwarded module enabled.

Steps to reproduce:

  1. Vanilla distribution 9.4.32.v20200930.
  2. Add --module=http-forwarded to start.ini.
  3. Start jetty.
  4. curl "http://localhost:8080/" --http1.0 -H 'Host:' -v

Same test with 9.4.31.v20200723 does not fail.

NPE_ForwardedRequestCustomizer.txt
@joakime
Copy link
Contributor

joakime commented Oct 13, 2020

This kind of overlaps with #5437 ...
It's where ForwardingRequestCustomizer still does things, even though the Forwarding headers might not be present.
The failure in this specific case is due to request.metadata not being filled out by the time ForwardingRequestCustomizer runs.

@joakime joakime self-assigned this Oct 13, 2020
@joakime joakime added the Bug For general bugs on Jetty side label Oct 13, 2020
@joakime joakime added this to To do in Jetty 9.4.33 via automation Oct 13, 2020
@joakime joakime moved this from To do to In progress in Jetty 9.4.33 Oct 13, 2020
joakime added a commit that referenced this issue Oct 13, 2020
@joakime
Issue #5443 - Forwarding Headers are optional
0b646ee 
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime mentioned this issue Oct 13, 2020
Merged
@joakime joakime linked a pull request Oct 13, 2020 that will close this issue
ForwardedRequestCustomizer behavior should not be applied to requests without forwarding headers #5445
Merged
@joakime
Copy link
Contributor

joakime commented Oct 13, 2020

Opened PR #5445

joakime added a commit that referenced this issue Oct 13, 2020
@joakime
Issue #5443 - Forwarding Headers are optional
f0681b3 
+ Simplify isSecure handling in customize.
+ Simplify handling of `Proxy-Ssl-Id` header.
+ Simplify handling of `Proxy-auth-cert` header.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Oct 13, 2020
@joakime
Issue #5443 - Forwarding Headers are optional
abdada0 
+ Improve / document implied secure scheme behaviors
  for both `Proxy-Ssl-Id` or `Proxy-auth-cert`

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Oct 13, 2020
@joakime
Issue #5443 - Forwarding Headers are optional
ea11030 
+ Additional tests for HTTP/1.0
+ Overly complex negative test cases for
   `X-Forwarded-Proto: http` and
   `X-Proxied-Https: off`
+ Failure testcase for `X-Proxied-Https: foo`

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Oct 13, 2020
@joakime
Issue #5443 - Forwarding Headers are optional
457025b 
Additional NPE safety checks.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Oct 13, 2020
@joakime
Issue #5443 - Forwarding Headers are optional
0721178 
The `X-Proxied-Https: off` case should have an implied port
not a hardcoded port.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Oct 13, 2020
@joakime
Issue #5443 - Forwarding Headers are optional
89dc16a 
Cleanup handling of forwarded.authority

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime
Copy link
Contributor

joakime commented Oct 13, 2020

PR #5445 merged (which fixes this)

@joakime joakime closed this as completed Oct 13, 2020
Jetty 9.4.33 automation moved this from In progress to Done Oct 13, 2020
@oyvindhorneland
Copy link
Author

Impressive! 👍

@joakime joakime changed the title Request without Host header fails with NullPointerException in ForwardedRequestCustomizer with Jetty 9.4.32.v20200930 Request without Host header fails with NullPointerException in ForwardedRequestCustomizer Oct 19, 2020
@joakime joakime mentioned this issue Oct 21, 2020
Closed
@joakime joakime mentioned this issue Nov 26, 2020
Closed
lucamilanesio pushed a commit to GerritCodeReview/gerrit that referenced this issue Nov 30, 2020
@syntonyze
Bump up jetty version to 9.4.33.v20201020
9b076ae 
This version, in particular, fixes the bug:
"Request without Host header fails with NullPointerException in
ForwardedRequestCustomizer" [1]

This bug caused Gerrit to throw a NullPointerException when serving
forwarded http/1.0 requests having no `Host` header set.

[1] jetty/jetty.project#5443

Bug: Issue 13752
Change-Id: I9f9f7df74f6d6c3996e044ba9883b2aa8951c209
@dependabot dependabot bot mentioned this issue Mar 15, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@joakime joakime

Labels
Bug For general bugs on Jetty side
Projects
No open projects
Jetty 9.4.33
  
Done
Milestone
No milestone
2 participants
@joakime @oyvindhorneland