On Mon, 24 Nov 2025 17:52:54 GMT, Daniel Jeliński <[email protected]> wrote:
> When checking for supported TLS versions, use SSLContext default parameters
> if the provided SSLParameters have no protocols configured.
>
> This fixes an issue where using SSLParameters with no protocols disabled the
> use of HTTP2 and HTTP3, even when these protocols were supported and enabled
> in the SSLContext.
>
> Modified the existing tests to additionally cover the case of empty
> SSLParameters. All tests continue to pass.
src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java line
472:
> 470: }
> 471: sslParams = requireNonNullElseGet(builder.sslParams,
> sslContext::getDefaultSSLParameters);
> 472: String[] protocols = sslParams.getProtocols();
This is already a pretty large ctor — can we rename this to `sslProtocols` to
ease the cognitive load?
src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java line
478:
> 476: if (protocols == null) {
> 477: // getDefaultSSLParameters.getProtocols should not return
> null
> 478: protocols = new String[0];
This can be simplified to:
Suggestion:
protocols =
requireNonNullElseGet(sslContext.getDefaultSSLParameters().getProtocols(), new
String[0]);
src/java.net.http/share/classes/jdk/internal/net/http/HttpConnection.java line
1:
> 1: /*
Thank you, this was a long-overdue clean up. 😍 I had an earlier attempt at this
in `HttpClientImpl::new`, but did not dare to extend its scope — was told,
_"There be dragons!"_ :rofl:
Curious: can we also clean up `QuicClient::requireTLS13` too? Personally, I'm
very much in favor of seeing only one occurrence of hardcoded `"TLSv1.2"` and
`"TLSv1.3"` string literals.
test/jdk/java/net/httpclient/TlsContextTest.java line 123:
> 121: }
> 122:
> 123: private void runTest(SSLContext context, Version version, String
> expectedProtocol, boolean setParams) throws Exception {
_Nit:_ `s/setParams/setEmptyParams/`?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28480#discussion_r2557696216
PR Review Comment: https://git.openjdk.org/jdk/pull/28480#discussion_r2557698850
PR Review Comment: https://git.openjdk.org/jdk/pull/28480#discussion_r2557710379
PR Review Comment: https://git.openjdk.org/jdk/pull/28480#discussion_r2557721461
