Configurable Unsafe Host Header Behaviors #9283
Conversation
+ Optional compliance that allowance duplicate host headers. Signed-off-by: Joakim Erdfelt <[email protected]>
+ Optional compliance that allows unsafe host headers. Signed-off-by: Joakim Erdfelt <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
joakime
added
Enhancement
Sponsored
Signed-off-by: Joakim Erdfelt <[email protected]>
jetty-util/src/test/java/org/eclipse/jetty/util/HostPortTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Joakim Erdfelt <[email protected]>
jetty-util/src/test/java/org/eclipse/jetty/util/HostPortTest.java
Outdated
Show resolved
Hide resolved
| public void testUnsafeAuthority(String authority) | ||
| { | ||
| HostPort hostPort = HostPort.unsafe(authority); | ||
| assertNotNull(hostPort); |
There was a problem hiding this comment.
We need to test more here. We need to test if the port was correctly extracted or not. So I think we need to pass in an expected port
There was a problem hiding this comment.
Many will be port 0, this is a test of unsafe(String authority) against totally broken authorities that the normal (safe) flows will reject and not allow.
We have a separate test for valid authorities that use unsafe(String authority) which does test for valid splits (host + port)
There was a problem hiding this comment.
In fact, in unsafe(String authority) we don't even test to make sure that the port is valid (within range 1 thru 65535).
It could be parsed as -80, or 333444555 as its unsafe.
There was a problem hiding this comment.
Basically everything about using unsafe(String authority) is unsafe, it can easily allow conditions later that will never work (you cannot create a URL or URI from them, or a valid Location header, etc)
There was a problem hiding this comment.
I'm ok with it being totally unsafe, but we need to consistently and predictably in our unsafe handling. Currently if it looks like an IPv6 address we don't check for a : before posting the port. If it doesn't look like IPv6 then we do check for the colon. I think we should check in both cases. No colon then no port.
There was a problem hiding this comment.
Reworked the test cases and the implementation in HostPort.
Signed-off-by: Joakim Erdfelt <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
joakime
added
the
Specification
joakime
changed the title
…x-documentation-operations-logging * upstream/jetty-12.0.x: (42 commits) fixed style cleanup TODOs for decoration Issue jetty#9300 - Rename RetainableByteBufferPool to ByteBufferPool Removed TODOs that will not be done. Rename Handler Nested & Collection (jetty#9305) fix surefire jpms configuration fix merge fix merge Bump maven.surefire.plugin.version from 3.0.0-M5 to 3.0.0-M8 (jetty#9255) Rename RetainableByteBufferPool to ByteBufferPool Fixed merge Fix jetty#9285 use possibly wrapper response for redirection (jetty#9286) Issue jetty#9293 - Jetty 12 - Relax JPMS dependencies (quic) (jetty#9307) Issue jetty#9293 - Jetty 12 - Relax JPMS dependencies (fcgi) (jetty#9306) Jetty 10 - Configurable Unsafe Host Header (jetty#9283) Issue jetty#9293 - Jetty 12 - Relax JPMS dependencies. (jetty#9296) Issue jetty#9293 - Jetty 12 - Relax JPMS dependencies. (jetty#9299) fix dependency add used dependency this dependency is used in test scope ...
HttpCompliance.DUPLICATE_HOST_HEADERSHttpCompliance.UNSAFE_HOST_HEADER