Review Request: signed module code restructuring/refactoring

[Sean Mullan]

On 5/22/12 7:19 PM, Mandy Chung wrote:
> On 5/18/2012 12:48 PM, Sean Mullan wrote:
>> I have restructured and refactored the signed module code. In particular I have
>> removed the ModuleFileVerifier and ModuleFileSigner interfaces (they really
>> didn't add any value) and made a few improvements here and there.
>>
>> webrev:
>> http://cr.openjdk.java.net/~mullan/jigsaw/webrevs/signed-module-refactor/webrev.00/
> 
> I went through the changes and look okay to me.
> 
> Signer.java
>    L96: is this a leftover from debugging?

No. It is useful to determine what is causing an error. Librarian has similar
code, but only prints a stack trace if a Command.Exception is thrown. I have
changed my code to do the same.

>    L322-332: storePassword.destroy() throws DestroyFailedException,
>    keyPassword.destroy() will not be called - should it handle
>    this exception case and destroy keyPassword?

Right - I think we need a try-with-passwords feature :)

I'll fix this but practically speaking this is probably not necessary. The
OpenJDK implementation of KeyStore.PasswordProtection.destroy() does not throw
DestroyFailedException (all it does is fill the char array with zeroes).

> SimpleLibrary.java
>    L1167: // XXX  - better to use '// ##' convention
> 
> You removed this comment - is it not applicable any more?
> 
> 1162  // ## Check policy - is signer trusted and what permissions
> 1163  // ## should be granted?

I was thinking this part through a bit and forgot to update it. It is still
applicable and is on my TODO list. I've adjusted the comment to reflect what I
am thinking at the moment:

// ## TODO: Check policy and determine if signer is trusted
// ## and what permissions should be granted.
// ## If there is no policy entry, show signers and prompt
// ## user to accept before proceeding.

--Sean