On Tue, 25 Nov 2025 11:08:00 GMT, Jaikiran Pai <[email protected]> wrote:
>> Daniel Jeliński has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Cleanup HttpClientImpl ctor
>> - Rename setParams
>
> src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java
> line 480:
>
>> 478: hasRequiredH3TLS =
>> Arrays.asList(sslProtocols).contains("TLSv1.3");
>> 479: // HTTP/2 MUST use TLS version 1.2 or higher for HTTP/2 over TLS
>> 480: hasRequiredH2TLS = hasRequiredH3TLS ||
>> Arrays.asList(sslProtocols).contains("TLSv1.2");
>
> Hello Daniel, I think if a `HttpClient` is configured with `sslParameters(new
> SSLParameters())`, then even with this new change in this PR, where we
> default the `sslProtocols` here to `new String[0]`, `hasRequiredH2TLS` would
> continue to return `false` right? Thus H2 is still disabled?
Hi Jaikiran, we default the protocols to
`sslContext.getDefaultSSLParameters().getProtocols()` which are documented to
be always non-null for compliant SSLContext implementations. The fallback to
`new String[0]` is only used for non-compliant SSLContexts.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28480#discussion_r2559789114
