Code Review Request: running signed modules with SecurityManager

[Sean Mullan]

On 5/27/10 2:42 PM, Mandy Chung wrote:
> On 05/26/10 15:02, Sean Mullan wrote:
>
>> Please review the webrev below which contains code changes that add
>> support for running signed modules with a SecurityManager.
>> http://cr.openjdk.java.net/~mullan/jigsaw/webrevs/SecurityManager2/webrev.00/
>>
> Some minor comments:
>
> ModuleClassLoader.java
> Should the defineModule method take a CodeSource parameter?

This is a really good question.

I actually considered that, and also changing the Module API to have a 
CodeSource object (i.e. Module.getCodeSource()). This would avoid maintaining 
the HashMap of ModuleIds to CodeSources in Loader.

I didn't think it was strictly necessary, but I guess I would like to get your 
opinion whether you think that would be a preferable design?

> Library.java:
> line 355: I wonder if the returned type should prefer Set<CodeSigner>
> to an array while the existing methods such as JarEntry.getCodeSigners
> return CodeSigner[]?

The reason I chose CodeSigner[] instead of Set<CodeSigner> is that this needs to 
be passed to the CodeSource constructor which currently takes a CodeSigner[]. So 
I wanted to avoid the Set.toArray conversion.

> Loader.java:
> line 196: Are we going to create a CodeSource object with a non-null
> URL some time?

Another good question. This is currently an open issue. In order to address this 
I think that we would need to retain the codebase URL of the module and store it 
in the library.

> SimpleLibrary.java
> line 540: When CNFE is thrown, I think AssertionError or InternalError
> is more appropriate.

ok.

>
> Packager.java
> line 237, 242 - you can use Files.delete(() that will throw IOException
> if it fails to delete the file.

ok.

Thanks,
Sean