On Thu, 4 Dec 2025 14:00:49 GMT, Sean Mullan <[email protected]> wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove null check to not assume key is returned
>
> src/java.base/share/classes/sun/security/ssl/DHasKEM.java line 79:
>
>> 77: // It registers Hybrid KeyPairGenerator, KeyFactory, and KEM
>> 78: // implementations for hybrid named groups as Provider services.
>> 79: private static class ProviderImpl extends Provider {
>
> This provider is not DH specific so doesn't fit right with being in this
> class. Move this `Provider` out into a separate class and call it
> `HybridProvider`, or alternatively put it inside the `Hybrid` class (so it
> would be `Hybrid.ProviderImpl`.) I know it also contains DHasKEM, which is
> not hybrid, but that is the only case, so I still think Hybrid is a suitable
> name, with a comment explaining there is one exception.
Ditto.
> src/java.base/share/classes/sun/security/ssl/DHasKEM.java line 83:
>
>> 81: private static final long serialVersionUID = 0L;
>> 82: private ProviderImpl() {
>> 83: super("InternalJCEDHasKEM", PROVIDER_VER,
>
> This provider supports more than DH, so suggest calling this
> "HybridAndDHAsKEM" or something like that.
I like this name suggestion.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2594528909
PR Review Comment: https://git.openjdk.org/jdk/pull/27614#discussion_r2594529563
