On Thu, 10 Apr 2025 11:37:23 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:
> RFC 9113 HTTP/2 mandates certain validation for HTTP headers; the HttpClient
> don't fully implement the described requirements.
>
> This PR adds the following validation:
> - pseudo-headers defined for requests are rejected in responses and push
> streams
> - pseudo-headers defined for responses are rejected in push promises
> - connection headers are rejected in responses and push streams
>
> Connection headers are still accepted in push promises; that's because some
> popular server implementations were found to echo the request headers in push
> promises, and when the original request was a HTTP/1 upgrade, the push
> promise could contain one or more headers that were prohibited in HTTP/2 but
> allowed in HTTP/1.
>
> An existing test was adapted to verify the handling of response headers. The
> modified test passes with this the changes in this PR, fails without them.
> Other tier1-3 tests continue to pass.
src/java.net.http/share/classes/jdk/internal/net/http/common/ValidatingHeadersConsumer.java
line 46:
> 44: REQUEST,
> 45: RESPONSE,
> 46: TRAILER
Do we need `TRAILER`, given we don't use it, AFAIU?
test/jdk/java/net/httpclient/http2/BadHeadersTest.java line 83:
> 81: of(entry(":status", "200"), entry("hello", "DE" + ((char) 0x7F)
> + "L")), // Bad byte in value
> 82: of(entry(":status", "200"), entry("connection", "close")),
> // Prohibited connection-specific header
> 83: of(entry(":status", "200"), entry(":scheme", "https")),
> // Request pseudo-header in response
Shouldn't we instead exhaustively test against all hard-coded header collection
in `ValidatingHeadersConsumer`, i.e., `PSEUDO_HEADERS` and `PROHIBITED_HEADERS`?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24569#discussion_r2037267873
PR Review Comment: https://git.openjdk.org/jdk/pull/24569#discussion_r2037262108
