[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