On Fri, 13 Nov 2020 04:57:12 GMT, Jamil Nimeh <jni...@openjdk.org> wrote:

> Hello all,
> This change brings in support for certificates with EdDSA keys (both Ed25519 
> and Ed448) allowing those signature algorithms to be used both on the 
> certificates themselves and used during the handshaking process for messages 
> like CertificateVerify, ServerKeyExchange and so forth.

test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 35:

> 33:  *     SunJSSE does not support dynamic system properties, no way to 
> re-use
> 34:  *     system properties in samevm/agentvm mode.
> 35:  */

Leading white spaces could be removed.  I may put this comment before the 
"@test" block to be consistent with other test cases.

test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 74:

> 72: 
> 73: public class TLSWithEdDSA extends SSLSocketTemplate {
> 74:     static final boolean DEBUG = false;

I may not use this filed.  The debug property could be specified in the "@run" 
tag if needed.

test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 81:

> 79:     static final String DEF_ALL_EE = 
> "EE_ECDSA_SECP256R1:EE_ECDSA_SECP384R1:" +
> 80:             "EE_ECDSA_SECP521R1:EE_RSA_2048:EE_EC_RSA_SECP256R1:" +
> 81:             "EE_DSA_2048:EE_DSA_1024:EE_ED25519:EE_ED448";

Why not use enum, array or collection directly?  Which is easy to read, I think.

test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 92:

> 90:     final SessionChecker serverChecker;
> 91:     final Class<? extends Throwable> clientException;
> 92:     final Class<? extends Throwable> serverException;

If no problem, I may declare the class fields and inner classes as private for 
safe.

test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 592:

> 590:     }
> 591: 
> 592:     private static void keyManagerTests(String keyStoreSpec, String 
> keyType,

Java method name is normally an action. What do you think if update to 
testKeyManager()?

test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 583:

> 581:         serverParameters.put(ParamType.CERTALIAS, "EE_ED25519");
> 582:         runtest(testFormat, isPeerEd25519, null, null, null);
> 583:         serverParameters.remove(ParamType.CERTALIAS);

I did not get the idea here.  Is there a special case in practice that use a 
similar key manger like the AliasKeyManager?

-------------

PR: https://git.openjdk.java.net/jdk/pull/1197

Reply via email to