Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

I’ve made that change at l.970 and updated webrev.02 in-place. Thanks. > On 12 Aug 2016, at 19:53, Sean Mullan wrote: > > Looks fine, although I would probably avoid calling checkX509Certs on line > 970 and just checking the cert right there to avoid creating the array

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

Looks fine, although I would probably avoid calling checkX509Certs on line 970 and just checking the cert right there to avoid creating the array which is not needed. --Sean On 08/12/2016 11:07 AM, Vincent Ryan wrote: I’ve moved the X.509 check to earlier in the code and reverted the changes

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

I’ve moved the X.509 check to earlier in the code and reverted the changes to the validateChain method. Updated webrev is at: http://cr.openjdk.java.net/~vinnie/8163503/webrev.02/ > On 10 Aug 2016, at 21:15, Sean Mullan wrote: > > On 08/10/2016 12:39 PM, Vincent

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

On 8/11/2016 4:15 AM, Sean Mullan wrote: > 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 >>

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

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.

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

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. An updated webrev is at:

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

Looks good. Thanks. Regards, Sean. On 10/08/16 17:39, Vincent Ryan wrote: I’ve updated the webrev to include your suggestion: http://cr.openjdk.java.net/~vinnie/8163503/webrev.01/ Thanks. On 10 Aug 2016, at 10:59, Seán Coffey wrote: It would be good if we can

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

You’re right. This same issue had been reported as an obscure JCK test failure. I created this new bug to clarify the issue. I’ve updated the webrev to include your suggestion: http://cr.openjdk.java.net/~vinnie/8163503/webrev.01/ Thanks. > On 10 Aug 2016, at 01:38, Weijun Wang

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

I’ve updated the webrev to include your suggestion: http://cr.openjdk.java.net/~vinnie/8163503/webrev.01/ Thanks. > On 10 Aug 2016, at 10:59, Seán Coffey wrote: > > It would be good if we can print the cert class type in the new exception if > the instanceof check

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

It would be good if we can print the cert class type in the new exception if the instanceof check fails. Regards, Sean. On 09/08/16 19: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

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

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

Re: [9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

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

[9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

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