Webrev updated at http://cr.openjdk.java.net/~weijun/8076190/webrev.03/.
Mostly spec changes. The test is enhanced a little to check for macAlg interop. > On Sep 24, 2018, at 11:15 PM, Sean Mullan <sean.mul...@oracle.com> wrote: > > Right, I understand their usage and the properties are well documented. My > comment is that if you just read the first sentence which is typically a > summary sentence, it gives no indication that some of these properties may > also be used when modifying a keystore. You have to read further to figure > that out, and that might be missed. Perhaps if you added a second sentence to > the first paragraph: "Several of the properties may also be used when > modifying an existing keystore." Then the next paragraph starts ... Good. > >>> The default alg values seem somewhat weak. Can we upgrade them or is there >>> a compatibility issue/risk? >> It will be addressed in a different RFE and is not related to migrating >> cacerts to password-less. >> I haven't studied it yet. Need to investigate how current releases of >> various tools (openssl, browsers...) support it. > > Ok. > > Still need to review PKCS12KeyStore, but here are some more comments: > > * java.security > > You should probably say that these properties are specific to the JDK PKCS12 > KeyStore implementation and may not be supported by other PKCS12 > implementations. > > What happens if the properties are not set or are set to the empty String? When not set, there is a default value. It happens to be the same as what the out-of-box java.security shows. When empty, there will be an error. Say, NumberFormatException, NoSuchAlgorithmException. > Are the algorithm names case-insensitive? Should be case-insensitive. For each one, I've specified it as "algorithm defined in the XYZ section of standard names". Is that enough to show it's case-insensitive? > > What if illegal values, or unknown algorithms are set for the properties? Are > they ignored? Do they throw an Exception? Or do they fallback to hard-coded > defaults? This behavior should be specified. Throw an exception. I cannot guarantee when it will be thrown so I just say "when it's used". > > * Passwordless > > Can you add some comments as to why openssl is needed? Aren't there some > tests you can still do if it is not there? And maybe add some comments in the > class description as to what the test is generally testing as it hard to > discern that from just scanning the code. Interoperability. I generate pkcs12 keystores from openssl using various settings and make sure keytool can read them, vice versa. Maybe I should move it to the infra test group. Thanks Max > > --Sean
