On Mon, 6 Jan 2025 12:48:25 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this change which fixes a regression that was > introduced in Java 22 release in the implementation of > `java.net.http.HttpClient`? This addresses > https://bugs.openjdk.org/browse/JDK-8346705. > > A client intiating a TLS handshake (through the `ClientHello`) can send a > "Server Name Indication" to tell the server, the name of the server it is > communicating with (https://www.rfc-editor.org/rfc/rfc6066#section-3). When a > `java.net.http.HttpClient` is establishing a (TLS) connection to serve a HTTP > request, it determines the SNI server name to send in the handshake. > > In Java 22, through https://bugs.openjdk.org/browse/JDK-8309910, we did an > (internal implementation detail) change to allow for the internal > implementation classes to keep track of what SNI server name was used for the > connection, so that that detail can be used for other purposes after the > connection has been established. That change wasn't meant to affect the logic > that determined the SNI server name to be sent across in the TLS handshake. > However, due to an oversight in the refactoring, a regression got introduced > which caused the HttpClient to no longer send a SNI server name for a certain > scenario (described below). > > The implementation of the HttpClient first looks at the request URI to > determine the SNI. If the request URI host is a IP address literal, then the > HttpClient implementation looks at the `HttpClient.sslParameters()` to see if > the application has configured any ServerName(s) through > `SSLParameters.setServerNames(...)`. If any are set, then the HttpClient uses > those ServerName(s) and sends them in the SNI, else it doesn't set the SNI. > On the other hand, if the URI host is not a IP address literal, then the > HttpClient implementation doesn't use any application configured > ServerName(s) and instead uses the request URI host as the SNI name and sends > it in the TLS handshake. > > The oversight in JDK-8309910 caused a regression where if the request URI > host is a IP address literal, then we no longer looked at the application > configured ServerName(s), thus causing the HttpClient to no longer set the > SNI server name in the TLS handshake. This can cause (and indeed does cause) > the TLS handshakes to fail if the server rejects the handshake due to the > missing SNI. > > The commit in this PR addresses this regression by making sure that the > HttpClient continues to behave the way it used to before JDK-8309910. A new > jtreg test case has been introduced to verify this existing be... This pull request has now been integrated. Changeset: 72f11149 Author: Jaikiran Pai <j...@openjdk.org> URL: https://git.openjdk.org/jdk/commit/72f1114909854aaed5d190d1c74a98527600a0c2 Stats: 268 lines in 3 files changed: 256 ins; 0 del; 12 mod 8346705: SNI not sent with Java 22+ using java.net.http.HttpClient.Builder#sslParameters Reviewed-by: dfuchs, michaelm ------------- PR: https://git.openjdk.org/jdk/pull/22927
