Issue #7250 - Update HostHeaderCustomizer for empty Host + no authority

[joakime]

Signed-off-by: Joakim Erdfelt [email protected]

[joakime]

There is an argument that we should not be setting or caring about the authority.

If we get a request like ...

GET foo:///12345 HTTP/1.1
Host:
Connection: close

That is a request without a declared authority, as specified by https://datatracker.ietf.org/doc/html/rfc7230#section-5.4
The foo scheme could have an implied authority as well.

For example file:///path/to/bar is a file scheme which says the implied authority is "localhost" (See https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.2)

If we reach a webapp in the above request that does HttpServletRequest.sendRedirect("/bar"), what does that mean to the Location: response header?
What about the HttpServletRequest.getServerName() ? (oddly, we get HttpServletRequest.getRequestURL() correct in the above example)

Question still remaining, do we force the Host header, even in the "empty host header" scenario?

[joakime]

Needs more tests for bad Host values.

Example:

Host: ":"
Host: ":43"
Host: -
Host: *
Host: -:88
Host: *.company.com

Should these be fixed by HostHeaderCustomizer as well? or treated as 400 Bad Request?
I'm kinda leaning towards 400 Bad Request, and having the new RejectNoAuthorityCustomizer renamed to RejectInvalidAuthorityCustomizer which also handles oddball authorities like above? WDYT?

We'll also need a new module for this new customizer.

[gregw]

I'd really REALLY prefer not to make any changes to HttpURI.

Instead focus on making a really good RejectInvalidAuthorityCustomizer that will solve the main concern of this PR.

Then by all means tweak the other Customizers to make them do a better job in the edge cases, but they don't need to be perfect if we have a good RIAC

[gregw]

What about IPv6, which can start with '['

[joakime]

handled in different path, and in uncommited code i mentioned.

[gregw]

Is this correct? Don't we just care that there is an Authority in eitehr the URI or Host header (and if both then they match)?
Or has this been handled for us already and any Host header already copied over to the URI authority?

[joakime]

Handled differently now, in HostPort.

[abahl]

This can cause NPE, isn't it? request.getHttpVersion() can return null.

[joakime]

The request shouldn't reach this customizer then, as I would expect the request to fail well before the customizers have been called.

[joakime]

This PR will be split into separate PRs.

[joakime]

This PR has now been broken down into ...

• PR Issue #7277 - Allow Request.getLocalName() and .getLocalPort() to be overridden #7283
• PR Issue #7269 - Refinement of HostPort behavior based on Specs #7279
• PR Issue #7250 - Correct HostHeaderCustomizer logic and add new RejectMissingAuthorityCustomizer #7292