On Tue, 21 Apr 2026 09:06:42 GMT, Volkan Yazici <[email protected]> wrote:
>> Per [RFC 6066 "3. Server Name Indication"], disallow IP literals in >> `SNIHostName::new`. >> >> While the following two call-sites could be simplified by removing IP >> literal checks, I've refrained from doing so because delegating some of the >> checks to an exception catching mechanism would impact the performance: >> >> sun.security.ssl.Utilities::rawToSNIHostName >> sun.net.www.protocol.https.HttpsClient::afterConnect >> >> [RFC 6066 "3. Server Name Indication"]: >> https://www.rfc-editor.org/rfc/rfc6066.html#page-6 >> >> --------- >> - [X] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Volkan Yazici has updated the pull request incrementally with one additional > commit since the last revision: > > Add `ofString` static factory method src/java.base/share/classes/javax/net/ssl/SNIHostName.java line 124: > 122: } > 123: > 124: private SNIHostName(String hostname, boolean strict) { Perhaps call it `allowIPLiteral`, instead of `strict`, to be more precise? src/java.base/share/classes/javax/net/ssl/SNIHostName.java line 132: > 130: IDN.USE_STD3_ASCII_RULES)) > 131: .getBytes(StandardCharsets.US_ASCII)); > 132: checkHostName(strict); With the newer relaxed rules for constructors which allows for additional methods to be called before the `super`, could this `checkHostName` method be made static and pass it the `hostname` to verify? That way, we could then call checkHostName before calling super? Something like the following (I haven't tried to see if it compiles): private SNIHostName(final String hostname, final boolean allowIPLiteral) { Objects.requireNonNull(hostname, "Server name value of host_name cannot be null"); // `IDN::toASCII` throws `IllegalArgumentException` on illegal input final String asciiHostName = IDN.toASCII(hostname, IDN.USE_STD3_ASCII_RULES); checkHostName(asciiHostName, allowIPLiteral); super(StandardConstants.SNI_HOST_NAME, asciiHostName.getBytes(StandardCharsets.US_ASCII)); } private static checkHostName(final String hostname, final boolean allowIPLiteral) { ... } src/java.base/share/classes/javax/net/ssl/SNIHostName.java line 443: > 441: // and not all call-sites have the sufficient `catch (IAE)` > safety net. > 442: // Hence, the strict checks enhancement is carried out as an > opt-in > 443: // feature. I think we should remove this code comment. This rationale I think is better captured in the CSR or the JBS issue of this change. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30747#discussion_r3151630196 PR Review Comment: https://git.openjdk.org/jdk/pull/30747#discussion_r3151624454 PR Review Comment: https://git.openjdk.org/jdk/pull/30747#discussion_r3151633854
