On 08/31/2011 09:17 PM, Xuelei Fan wrote:
I understand the code, fine to me. But the loadKeyStore() method looks
really ugly and lazy. :-)

It's ugly, but not very lazy.

Anyway, I'm going to putback this version since you already said fine.


Just for your reference in the inline comments.

...


Well, it looks more correct, but is complicated in 2 senses:

1. ~/.keystore and user-specified -keystore are not treated the same.
You can ignore ~/.keystore, but if a user-specified -keystore does not
exist, it's an error.

We can handle the logic simply at the following blok, right?
1562 if (!nullStream&&  keyStoreName == null) {
1563     keyStoreName = System.getProperty("user.home") + File.separator
1564         + ".keystore";
+        // check file existence, ignore it if non-exist
1565 }

2. signing and verification have different behaviors on exception
handling. See above.

We don't need to make more significant update in other blocks because of
we have ignore the non-exist keystore, right? If no required private
key, the following steps will throw the expected exception.

If you ignore non-existing ~/.keystore, there will be no exception, and store will be either null or uninitialized. Then when it's used, NPE or KeyStoreException will be thrown and the user is confused. So you still need to check it earlier.

Thanks
Max


Or is there any other thing that I missed?

Thanks,
Xuelei

Reply via email to