On Mon, 7 Oct 2024 16:57:56 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> no need for this when there is no ambiguity
>
> src/java.base/share/classes/sun/security/provider/NamedKEM.java line 72:
>
>> 70: throws InvalidAlgorithmParameterException,
>> InvalidKeyException {
>> 71: if (spec != null) {
>> 72: throw new InvalidAlgorithmParameterException("No params
>> needed");
>
> Could you make this message more helpful? Ex: "The ML-KEM-768 algorithm does
> not take any parameters", where "ML-KEM-768" is the alg+param name in use
> (may need a new protected method to obtain that).
I think we can just say "ML-KEM does not take any parameters". Otherwise it
gives out a false hint that ML-KEM-768 does not because it already has a
parameter set. The parameters in this method is not the parameters for the key
but for the KEM algorithm itself. This is also the same for
`NamedSignature::engineSetParameter`. It's even important there because one day
when we decide to support PreHash ML-DSA we will need parameters.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21167#discussion_r1790666004
