On Wed, 9 Apr 2025 10:57:45 GMT, Mikhail Yankelevich <[email protected]>
wrote:
>> Hi,
>>
>> I would like to request a review for the fix of JDK-8350661. In this fix, we
>> translate the native PKCS 11 error code into an
>> `InvalidAlgorithmParameterException`, as documented in the `KDF::deriveKey`
>> API. With that said, different PKCS 11 libraries may throw different errors
>> and may even (in theory) delay the error until the key is used, as _SunJCE_
>> does. I believe that this is an improvement but further adjustments may be
>> needed in the future.
>>
>> No regressions observed in `test/jdk/sun/security/pkcs11/KDF/TestHKDF.java`.
>>
>> Thanks,
>> Martin.-
>
> test/jdk/sun/security/pkcs11/KDF/TestHKDF.java line 619:
>
>> 617: k.deriveKey("AES", HKDFParameterSpec.ofExtract()
>> 618: .thenExpand(null, 31));
>> 619: throw new Exception("No exception thrown.");
>
> I think this throw is unnecessary, you can just call this code straight on
> line 619 and remove the catch block below
>
> reportTestFailure(new Exception("Derivation of an AES key of " +
> "invalid size (31 bytes) expected to throw " +
> "InvalidAlgorithmParameterException.", e));
>
>
> What do you think?
The catch block would still be needed because another exception (such as
`ProviderException`, as is the case now) might be thrown and we want to make
sure that it does not occur.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24526#discussion_r2035384667
