On Thu, 8 Apr 2021 00:01:57 GMT, Weijun Wang <[email protected]> wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> update with review comments
>
> src/java.base/share/classes/sun/security/tools/keytool/CertAndKeyGen.java
> line 88:
>
>> 86: */
>> 87: public CertAndKeyGen (String keyType, String sigAlg, String
>> providerName)
>> 88: throws NoSuchAlgorithmException, NoSuchProviderException
>
> Indent the line 8 spaces further.
Done.
> test/jdk/sun/security/tools/keytool/GenKeyPairSigner.java line 96:
>
>> 94:
>> 95: Certificate[] certChain = kstore.getCertificateChain("e1");
>> 96: if (certChain.length != 2) {
>
> Try using `Asserts` class in `/test/lib` to make code simpler. Also, why not
> throw an exception but call `System.exit(1)`? We usually do not call this
> method in a test because the test framework must take great care so that
> itself does not get terminated.
Changed to throw the exception for errors. Meanwhile, the test is pretty
straightforward/simple, and using if comparison should serve its testing need
and it does not make the code complicated.
> test/jdk/sun/security/tools/keytool/GenKeyPairSigner.java line 299:
>
>> 297: System.exit(1);
>> 298: }
>> 299:
>
> Since you are here, you can check if the new entry is indeed protected by the
> new key password.
In testSignerJKS() has made sure that the new entry created with new key
password can *only* be accessed when a correct key password is provided in
order to retrieve the corresponding signer’s private key.
-------------
PR: https://git.openjdk.java.net/jdk/pull/3281
