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

Sean Mullan sean.mullan at oracle.com
Fri May 22 15:26:54 UTC 2015


On 05/21/2015 06:02 PM, Thomas Lußnig wrote:
> 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.

Alternatively, if there is no compatModeEnabled, there is no need to 
try/catch so line 203 should probably be:

  203         if (stream == null || !compatModeEnabled) {

and then you can remove the check for compatModeEnabled in the try/catch 
block.

--Sean

> 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