On Wed, 13 May 2026 10:18:32 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 19 additional
>> commits since the last revision:
>>
>> - Merge remote-tracking branch 'upstream/master' into sni
>> - Ensure `encoded` is copied
>> - Add tests and docs for UTF-8 vs. ASCII discrepancy
>> - Remove `{@return` tags
>> - Add reference to RFC 5280
>> - Simplify "ASCII or UTF-8" with just "UTF-8"
>> - Revert javadoc changes from deprecated methods
>> - Do the forgotten update
>> - Improve wording and HTML rendering
>> - Merge remote-tracking branch 'upstream/master' into sni
>> - ... and 9 more: https://git.openjdk.org/jdk/compare/a6250f7a...0e562368
>
> src/java.base/share/classes/javax/net/ssl/SNIHostName.java line 134:
>
>> 132:
>> 133: /**
>> 134: * Returns an {@code SNIHostName} using the specified hostname.
>
> Maybe "from" instead of "using' here.
Changed as requested in 0d229310f98.
> src/java.base/share/classes/javax/net/ssl/SNIHostName.java line 139:
>
>> 137: * href="http://www.ietf.org/rfc1123.txt";>RFC 1123</a> and <a
>> 138: * href="http://www.ietf.org/rfc5280.txt";>RFC 5280</a>), which
>> is
>> 139: * either an ASCII-encoded hostname or an {@linkplain IDN
>> Internationalized
>
> I think it would better to use the more restrictive "that is either" rather
> ", which is either" here. The repeated use of "hostname" could be dropped too
> to give you:
>
> "A valid SNI hostname is a DNS hostname (see RFC 1123 and RFC 5280) that is
> either ASCII-encoded or an Internationalized Domain Name (IDN)."
Changed as requested in 849d2bddc6e.
> src/java.base/share/classes/javax/net/ssl/SNIHostName.java line 173:
>
>> 171: * a decision depending on its context; e.g., propagate the
>> failure, or
>> 172: * report the issue and skip {@linkplain
>> SSLParameters#setServerNames(List)
>> 173: * the SNI server name configuration}.
>
> The wording in this API note is bit unusual. Is this API note needed? It is
> useful to suggest that the SNI server name configuration should be skipped?
Corpus search had revealed that many use sites did not catch `IAE`. Recall that
this was the reason we've decided to introduce new static factory methods
instead of introducing stricter checks to existing constructors. This API note
was intended to guide users in the desired usage pattern:
1. Give leeway to further future restrictions.
2. Thrown `IAE` can be used to skip SNI server name configuration, instead of
failing the entire TLS/SSL configuration. This is also what we do in
`sun.security.ssl.Utilities#rawToSNIHostName`.
Maybe @artur-oracle can weigh in if this problem also resonates with him and
this API note does indeed help with mitigating the problem?
> There's an error in "a valid a DNS".
>
> Maybe it would be clearer to split the long sentence, e.g. "This method
> decodes the bytes into a hostname string. The hostname is a DNS hostname ...".
Changed as requested in af14d5a3056.
> Can this method be specified to work the same as ofHostName if called with
> the decoded hostname?
While I can see this approach's appeal, I'd not go that way: `ofEncoded` does
**not** _"work the same as ofHostName if called with the decoded hostname"_.
Recall the difference how they populate `encoded` field:
https://github.com/vy/jdk/blob/af14d5a30565c9b61ac6d1f8872a611ac1104bca/src/java.base/share/classes/javax/net/ssl/SNIHostName.java#L275-L282
Please let me know if you still want me to update the specification in this
direction.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30747#discussion_r3246669604
PR Review Comment: https://git.openjdk.org/jdk/pull/30747#discussion_r3246669265
PR Review Comment: https://git.openjdk.org/jdk/pull/30747#discussion_r3246669997
PR Review Comment: https://git.openjdk.org/jdk/pull/30747#discussion_r3246670517
