[8u] request for review: 8062552 Support keystore type detection for JKS and PKCS12 keystores

Thomas Lußnig openjdk at suche.org
Thu May 21 22:02:22 UTC 2015


Hi,

most of it look ok for me, but in
"http://cr.openjdk.java.net/~vinnie/8062552/webrev.01/src/share/classes/sun/security/util/KeyStoreDelegator.java.patch"
i found in the
method engineLoad the exception handling an bit ugly.

+                } catch (Exception e) {
+
+                    // incorrect password
+                    if (e instanceof IOException &&
+                        e.getCause() instanceof UnrecoverableKeyException) {
+                        throw (IOException)e;
+                    }
+
+                    try {
+                        // Ignore secondary keystore when no compatibility mode
+                        if (!compatModeEnabled) {
+                            throw e;
+                        }

i would have expected:

+                } catch (Exception e) {
+                    if (!compatModeEnabled) {
+                        throw e;
+                    }
+
+                    // incorrect password
+                    if (e instanceof IOException &&
+                        e.getCause() instanceof UnrecoverableKeyException) {
+                        throw (IOException)e;
+                    }
+
+                    try {
+                        // Ignore secondary keystore when no compatibility mode

because if no compatModeEnabled i would expect that the exception if transparently thrown.
And since JDK8 this is allowed tho throw an Exception if it was catched and can only by an declared one.
Also this only compare an boolean instead of two instanceof + additional exception handling.

Gruß Thomas




More information about the security-dev mailing list