On 08/10/2016 12:39 PM, Vincent Ryan wrote:
Yes they could be merged but the first loop iterates over all the certs and the 
second one iterates over all but the final cert.
And the special case of a 1-cert chain also needs to be handled. I think it’s a 
little clearer to leave them separate.

Agreed, but it's probably better to check these earlier and bail out if they are not all X509Certificates, for example, right at the beginning of engineSetKeyEntry. There is no need to proceed with decoding keys, etc since it is already a violation of the supported API parameters.

--Sean


An updated webrev is at:
  http://cr.openjdk.java.net/~vinnie/8163503/webrev.01/

Thanks.

On 10 Aug 2016, at 02:04, Xuelei Fan <xuelei....@oracle.com> wrote:

The for loop at line 1507 and 1520 may be merged together.

Xuelei

On 8/10/2016 8:38 AM, Weijun Wang wrote:
I thought I've seen this webrev before.

Why not just throw a KeyStoreException in validateChain()?

--Max

On 8/10/2016 2:14, Vincent Ryan wrote:
Please review this fix to improve the error handling for attempts to
store a Certificate object in PKCS12 keystore.
The PKCS12 keystore implementation supports storing only
X509Certificate objects but the KeyStore API allows Certificate objects.
This fix rejects attempts to store non-X.509 certificates and throws a
KeyStoreException.

Thanks.

Bug: https://bugs.openjdk.java.net/browse/JDK-8163503
Webrev: http://cr.openjdk.java.net/~vinnie/8163503/webrev.00/




Reply via email to