On Thu, 1 Apr 2021 23:36:04 GMT, Hai-May Chao <hc...@openjdk.org> wrote:
>> Please review the changes that adds the -signer option to keytool >> -genkeypair command. As key agreement algorithms do not have a signing >> algorithm, the specified signer's private key will be used to sign and >> generate a key agreement certificate. >> CSR review is at: https://bugs.openjdk.java.net/browse/JDK-8264325 > > Hai-May Chao has updated the pull request incrementally with one additional > commit since the last revision: > > update with review comments Only a few minor comments. Approved. src/java.base/share/classes/sun/security/tools/keytool/Main.java line 1978: > 1976: keypair.getPublicKeyAnyway(), > 1977: signerSubjectKeyId); > 1978: } else { No need for an if-else block? `signerSubjectKeyId` is null in this case. src/java.base/share/classes/sun/security/tools/keytool/Main.java line 2013: > 2011: x500Name}; > 2012: System.err.println(form.format(source)); > 2013: } Either put the declaration of `source` outside the if-else block and move the println line outside; or, move the declaration of `form` inside. It looks confusing that `form` is declared outside and `source` inside. src/java.base/share/classes/sun/security/tools/keytool/Resources.java line 310: > 308: "Generating {0} bit {1} key pair and self-signed > certificate ({2}) with a validity of {3} days\n\tfor: {4}"}, > 309: > {"Generating.keysize.bit.keyAlgName.key.pair.and.a.certificate.sigAlgName.issued.by.an.entry.signerAlias.with.a.validity.of.validality.days.for", > 310: "Generating {0} bit {1} key pair and a certificate ({2}) > issued by an entry <{3}> with a validity of {4} days\n\tfor: {5}"}, I feel "issued by <{3}>" sounds more normal. No need to say "an entry". You decide it. ------------- Marked as reviewed by weijun (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3281
