On Mon, 24 Nov 2025 20:45:27 GMT, Volkan Yazici <[email protected]> wrote:
>> Add guards against `HttpHeaders::firstValueAsLong` failures.
>> `H3MalformedResponseTest` is overhauled – migrated to JUnit, reinforced with
>> exception type tests, etc. – but not all `firstValueAsLong` changes are
>> verified with new additional tests. `test/jdk/java/net/httpclient/` tests
>> still do pass.
>
> Volkan Yazici has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Address review remarks
src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java line
338:
> 336: // expect-continue reads headers and body twice.
> 337: // if we reach here, we must reset the headersReader state.
> 338: finally {
Is this `finally` necessary? The `ProtocolException` is supposed to kill the
connection anyway, so its internal state shouldn't matter much.
If it does matter, please move the `finally` to the closing brace of the
`catch`; it looks odd now.
src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java line
374:
> 372: // Check `Transfer-Encoding`
> 373: var transferEncoding =
> headers.firstValue("Transfer-Encoding").orElse(null);
> 374: if (transferEncoding != null) {
use `isPresent` instead of `orElse(null) != null`
src/java.net.http/share/classes/jdk/internal/net/http/Stream.java line 1822:
> 1820: }
> 1821:
> 1822: private static int readStatusCode(HttpHeaders headers, String
> errorPrefix) throws ProtocolException {
There's a similar piece of code for HTTP3
[here](https://github.com/openjdk/jdk/blob/b491c9bc98a8da0a1d913e85673087e1b929cb3d/src/java.net.http/share/classes/jdk/internal/net/http/Http3Stream.java#L610-L624);
do you think you could merge them?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28431#discussion_r2559547910
PR Review Comment: https://git.openjdk.org/jdk/pull/28431#discussion_r2559710182
PR Review Comment: https://git.openjdk.org/jdk/pull/28431#discussion_r2559813988
