On Wed, 6 May 2026 07:42:15 GMT, Alan Bateman <[email protected]> wrote:
>> Volkan Yazici has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 10 additional
>> commits since the last revision:
>>
>> - Merge remote-tracking branch 'upstream/master' into sni
>> - Apply review feedback
>> - Fix variable shadowing issue in `SNIHostName`
>> - Enrich tests
>> - Use `sun.security.x509.DNSName` in strict checks
>> - Merge remote-tracking branch 'upstream/master' into sni
>> - Improve deprecation message
>> - Big facelift
>> - Add `ofString` static factory method
>> - Disallow IP literals in `SNIHostName::new`
>
> src/java.base/share/classes/javax/net/ssl/SNIHostName.java line 105:
>
>> 103: * StandardCharsets#US_ASCII ASCII}.
>> 104: * {@link IDN#toASCII(String, int) IDN.toASCII(hostname,
>> IDN.USE_STD3_ASCII_RULES)}
>> 105: * is used to enforce the restrictions on ASCII characters in
>> hostnames (see
>
> This is confusing when rendered because it reads the space between ASCII and
> IDN.toASCII isn't very obvious (if you generate the API docs then you'll see
> what I mean). Maybe change it to "The IDN.toASCII(..) method is used to
> enforce .." and that will avoid have the captialized words running into each
> other.
Fixed using `The IDN.toASCII(...) method` in 3dedf161fd3.
> src/java.base/share/classes/javax/net/ssl/SNIHostName.java line 179:
>
>> 177: * depending on their context; e.g., propagate the failure, or
>> report the
>> 178: * issue and skip {@linkplain SSLParameters#setServerNames(List)
>> the SNI
>> 179: * server name configuration}.
>
> Instead of "Users are expected" then it might be clearer to say "Code using
> this method is expected to ..."
Fixed in 3dedf161fd3.
> src/java.base/share/classes/javax/net/ssl/SNIHostName.java line 208:
>
>> 206: * constitute a DNS hostname, which is either an ASCII-encoded
>> hostname or
>> 207: * an {@linkplain IDN Internationalized Domain Name (IDN)}. A
>> decoded
>> 208: * hostname string is considered illegal if it:
>
> "Once the specific value is decoded ..." is confusing. I think you'll need to
> re-phase this to say the input is decoded into a host name string so there is
> no ambiguity on who and when the bytes are decoded.
In 3dedf161fd3, re-phrased it as follows:
> The specified byte array gets decoded into a hostname string that is
> required to be a valid a DNS hostname, which is either an ASCII-encoded ...
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30747#discussion_r3194415199
PR Review Comment: https://git.openjdk.org/jdk/pull/30747#discussion_r3194416458
PR Review Comment: https://git.openjdk.org/jdk/pull/30747#discussion_r3194431276
