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 >> 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. > Good!
Xuelei > --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/ >>>>> >>>>> >>> >>
