On Sat, 6 Dec 2025 07:36:16 GMT, Bradford Wetmore <[email protected]> wrote:
> I know we haven't been consistent in the visibility of the internal JSSE
> classes (and members therein), but many (all?) of the new classes (including
> nested classes: e.g. Hybrid.*) could be package-private/final (or even
> private) instead of public.
>
> I'm not suggesting going through and doing an overhaul of the
> `sun.security.ssl` package, just the new ones.
>
> Also, you changed the files while I was reviewing, so several of my comments
> are considered outdated. One still applies in the new location, so I'll readd
> it.
Went through the new files. Sorry for changing the files.
> src/java.base/share/classes/sun/security/ssl/DHasKEM.java line 259:
>
>> 257: }
>> 258:
>> 259: public static class HybridService extends Provider.Service {
>
> Shouldn't this be moved to `HybridProvider.java`?
Yes, moved.
> src/java.base/share/classes/sun/security/ssl/HybridProvider.java line 57:
>
>> 55: // The order of shares in the concatenation for group name
>> 56: // X25519MLKEM768 has been reversed. This is due to IETF
>> 57: // historical reasons.
>
> Can we change this to something like "as per the current draft RFC?"
>
> "historical reasons" is too vague. The draft/RFC is the real reason.
Changed the comment as suggested.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/27614#issuecomment-3620719989
PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2595151301
PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2595151228
