On Tue, 2 Dec 2025 11:14:19 GMT, Volkan Yazici <[email protected]> wrote:
>> Sergey Chernyshev has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - addressed more review comments >> - addressed review comments > > src/java.base/share/classes/sun/net/www/protocol/https/HttpsClient.java line > 476: > >> 474: // host has been set previously for SSLSocketImpl >> 475: if (!(s instanceof SSLSocketImpl) && >> 476: !IPAddressUtil.isIPv4LiteralAddress(host) && > > This introduces a new behavior for both IPv4 and IPv6, yet we only verify the > IPv6 behavior. Shouldn't we also verify that when the provided host is an > IPv4 literal, `setServerNames()` is left untouched? @vy The test excercises the same code path as in the BCJSSE case, that throws an exception on non-LDH symbols. Segments of IPv4 literal adresses are all LDH, so they do not trigger any exception. Adding an IPAddressUtil.isIPv4LiteralAddress() check in the above condition is purely to mirror SSLSocketImpl behavior, as I thought initially. On the other hand, should we then add a negative test with a certificate that doesn't have a SAN extension (or the 127.0.0.1 ipv4 address in it), that should fail in the HostnameVerifier when the 'https://127.0.0.1' is requested? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28577#discussion_r2580917249
