Update to address review comments from Weijun

Author: valeriepeng

src/java.base/share/classes/sun/security/rsa/RSAKeyFactory.java

@@ -43,14 +43,14 @@
  * between the following:
  *
  * For public keys:
- *  . PublicKey with an X.509 encoding
+ *  . RSA PublicKey with an X.509 encoding
  *  . RSA PublicKey with an PKCS#1 encoding
  *  . RSAPublicKey
  *  . RSAPublicKeySpec
  *  . X509EncodedKeySpec
  *
  * For private keys:
- *  . PrivateKey with a PKCS#8 encoding
+ *  . RSA PrivateKey with a PKCS#8 encoding
  *  . RSA PrivateKey with a PKCS#1 encoding
  *  . RSAPrivateKey
  *  . RSAPrivateCrtKey
@@ -381,7 +381,8 @@ protected <T extends KeySpec> T engineGetKeySpec(Key key, Class<T> keySpec)
         try {
             // convert key to one of our keys
             // this also verifies that the key is a valid RSA key and ensures
-            // that the encoding is X.509/PKCS#8 for public/private keys
+            // that the encoding is X.509/PKCS#8 or PKCS#1 for public/private
+            // keys
             key = engineTranslateKey(key);
         } catch (InvalidKeyException e) {
             throw new InvalidKeySpecException(e);

src/java.base/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java

@@ -98,7 +98,9 @@ public static RSAPrivateKey newKey(KeyType type, String format,
                 (key.getCrtCoefficient().signum() == 0)) {
                 return new RSAPrivateKeyImpl(key.type, key.keyParams,
                     key.getModulus(), key.getPrivateExponent());
-            } else return key;
+            } else {
+                return key;
+            }
         case "PKCS#1":
             try {
                 BigInteger[] comps = parseASN1(encoded);

test/jdk/sun/security/pkcs11/rsa/TestKeyFactory.java

@@ -153,10 +153,9 @@ private static void testKey(Key key1, Key key2) throws Exception {
             }
         }
         // only compare encodings if keys are of the same format
-        if (key1.getFormat().equals(key2.getFormat())) {
-            if (!Arrays.equals(key1.getEncoded(), key2.getEncoded())) {
-                throw new Exception("Encodings not equal");
-            }
+        if (key1.getFormat().equals(key2.getFormat()) &&
+            !Arrays.equals(key1.getEncoded(), key2.getEncoded())) {
+            throw new Exception("Encodings not equal");
         }
     }
 

test/jdk/sun/security/rsa/TestKeyFactory.java

@@ -81,12 +81,12 @@ public class TestKeyFactory {
         "QKBW1TS0/2iB9zNkFMj5/+h7l2oqTT7sSQIDAQAB";
 
 
-    private static final PrivateKey CUSTOM_PRIV;
-    private static final PublicKey CUSTOM_PUB;
+    private static final PrivateKey P1_PRIV;
+    private static final PublicKey P1_PUB;
 
     static {
         byte[] encodedPriv = Base64.getDecoder().decode(PKCS1_PRIV_STR);
-        CUSTOM_PRIV = new PrivateKey() {
+        P1_PRIV = new PrivateKey() {
             @Override
             public String getAlgorithm() {
                 return "RSA";
@@ -102,7 +102,7 @@ public byte[] getEncoded() {
             }
         };
         byte[] encodedPub = Base64.getDecoder().decode(PKCS1_PUB_STR);
-        CUSTOM_PUB = new PublicKey() {
+        P1_PUB = new PublicKey() {
             @Override
             public String getAlgorithm() {
                 return "RSA";
@@ -145,31 +145,30 @@ private static void testKey(Key key1, Key key2) throws Exception {
             }
         }
         // skip equals check when key1 is custom key
-        if (key1 != CUSTOM_PRIV && key1 != CUSTOM_PUB) {
+        if (key1 != P1_PRIV && key1 != P1_PUB) {
             if (!key1.equals(key2)) {
                 throw new Exception("Keys not equal");
             }
         }
 
         // only compare encodings if keys are of the same format
-        if (key1.getFormat().equals(key2.getFormat())) {
-            if (!Arrays.equals(key1.getEncoded(), key2.getEncoded())) {
-                throw new Exception("Encodings not equal");
-            }
+        if (key1.getFormat().equals(key2.getFormat()) &&
+            !Arrays.equals(key1.getEncoded(), key2.getEncoded())) {
+            throw new Exception("Encodings not equal");
         }
     }
 
     private static void testPublic(KeyFactory kf, PublicKey key)
             throws Exception {
-        System.out.println("Testing " + (key == CUSTOM_PUB? "PKCS#1" : "") +
+        System.out.println("Testing " + (key == P1_PUB? "PKCS#1" : "") +
             " public key...");
 
         PublicKey key2 = (PublicKey)kf.translateKey(key);
         KeySpec rsaSpec = kf.getKeySpec(key, RSAPublicKeySpec.class);
         PublicKey key3 = kf.generatePublic(rsaSpec);
         KeySpec x509Spec = kf.getKeySpec(key, X509EncodedKeySpec.class);
         PublicKey key4 = kf.generatePublic(x509Spec);
-        if (key != CUSTOM_PUB) {
+        if (key != P1_PUB) {
             testKey(key, key);
         }
         testKey(key, key2);
@@ -185,14 +184,14 @@ private static void testPublic(KeyFactory kf, PublicKey key)
 
     private static void testPrivate(KeyFactory kf, PrivateKey key)
             throws Exception {
-        System.out.println("Testing " + (key == CUSTOM_PRIV? "PKCS#1" : "") +
+        System.out.println("Testing " + (key == P1_PRIV? "PKCS#1" : "") +
             " private key...");
         PrivateKey key2 = (PrivateKey)kf.translateKey(key);
         KeySpec rsaSpec = kf.getKeySpec(key, RSAPrivateCrtKeySpec.class);
         PrivateKey key3 = kf.generatePrivate(rsaSpec);
         KeySpec pkcs8Spec = kf.getKeySpec(key, PKCS8EncodedKeySpec.class);
         PrivateKey key4 = kf.generatePrivate(pkcs8Spec);
-        if (key != CUSTOM_PRIV) {
+        if (key != P1_PRIV) {
             testKey(key, key);
         }
         testKey(key, key2);
@@ -240,8 +239,8 @@ public static void main(String[] args) throws Exception {
             }
         }
         // repeat the test w/ PKCS#1 RSA Private Key
-        test(kf, CUSTOM_PRIV);
-        test(kf, CUSTOM_PUB);
+        test(kf, P1_PRIV);
+        test(kf, P1_PUB);
 
         long stop = System.currentTimeMillis();
         System.out.println("All tests passed (" + (stop - start) + " ms).");