On Wed, 31 May 2023 13:00:15 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Sibabrata Sahoo has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8308711: Comment addressed
>
> test/jdk/javax/crypto/KEM/GenLargeNumberOfKeys.java line 45:
>
>> 43: * X448 produce keysize of 64 bytes which is larger in it's class
>> 44: * secp521r1 produce keysize of 64 bytes which is larger in it's
>> class
>> 45: */
>
> I don't quite understand what the comment mean.
Removed this comment.
> test/jdk/javax/crypto/KEM/GenLargeNumberOfKeys.java line 64:
>
>> 62: private static KeyPair genKeyPair(String algo, String curveId)
>> throws Exception {
>> 63: KeyPairGenerator kpg = KeyPairGenerator.getInstance(algo);
>> 64: kpg.initialize(new ECGenParameterSpec(curveId));
>
> Although `ECGenParameterSpec` is a `NamedParameterSpec`, we usually don't
> create XDH keys using it. How about following the same style in `KemTest` and
> only call `initialize` when it's EC. For XDH, just use `X448` as the `algo`
> and there is no need to initialize it.
Done.
> test/jdk/javax/crypto/KEM/KemTest.java line 129:
>
>> 127: Asserts.assertEQ(encT.encapsulationSize(),
>> decT.encapsulationSize());
>> 128: Asserts.assertEQ(encT.secretSize(),
>> enc.key().getEncoded().length);
>> 129: Asserts.assertEQ(encT.secretSize(), decT.secretSize());
>
> Since the decapsulator also has `secretSize` and `encapsulationSize` methods,
> you can add some more lines testing the output of them as well.
Done.
> test/jdk/javax/crypto/KEM/KemTest.java line 195:
>
>> 193: Callable<KEM.Encapsulator> task = () -> {
>> 194: return kem.newEncapsulator(kp.getPublic());
>> 195: };
>
> Just use `() -> kem.newEncapsulator(kp.getPublic())`. Same on lines 229, 262,
> 300.
Done.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14113#discussion_r1212714688
PR Review Comment: https://git.openjdk.org/jdk/pull/14113#discussion_r1212715230
PR Review Comment: https://git.openjdk.org/jdk/pull/14113#discussion_r1212715626
PR Review Comment: https://git.openjdk.org/jdk/pull/14113#discussion_r1212715868
