On 12/16/2014 07:12 PM, Vincent Ryan wrote:
I’ve generated a new webrev to address your comments below:
http://cr.openjdk.java.net/~vinnie/8044445/webrev.04/
Thanks.
On 16 Dec 2014, at 21:11, Sean Mullan <sean.mul...@oracle.com> wrote:
Here are my comments on the latest webrev:
* General
- for the engineProbe methods, if a DataInputStream is not specified, I think
you should wrap the stream in a DataInputStream, rather than returning false.
KeyStore.getInstance(File,…) always supplies a DataInputStream to
KeyStoreSpi.engineProbe
so it is not necessary to wrap.
That's an implementation detail though. Another vendor's implementation
of the new KeyStore.getInstance methods may not do that. If you want to
guarantee that, you should change the InputStream parameter of the
engineProbe method to a DataInputStream. Unless you think engineProbe
may be called/used in other circumstances where a DataInputStream is not
appropriate, which in that case you change the subclasses to do an
instanceof and wrap it.
* keytool/Main
- can you explain more about why the provider is ignored on lines 812-817,
1919-1925? This doesn't seem like it is compliant with keytool -- if a provider
is specified, it should be used.
The probing mechanism is better equipped to determine the exact keystore type.
For example, if an application specifies no -storetype option then it
implicitly gets the
default keystore type which now differs between JDK 9 and earlier releases.
The new compatibility mode can handle this scenario so that the application
doesn’t
break. However, in compatibility mode the instantiated keystore type might not
match
the keystore format.
Whereas when probing is employed the instantiated keystore type always matches
the keystore format.
Ok, but I think you should do that only if both a provider and storetype
are not specified. If the -providerName option is specified, you need to
honor that. It looks like the code is not doing that right now.
--Sean
