Code Review Request: running signed modules with SecurityManager
mark.reinhold at oracle.com
mark.reinhold at oracle.com
Fri Jun 4 11:31:48 PDT 2010
> Date: Tue, 01 Jun 2010 09:50:21 -0400
> From: sean.mullan at oracle.com
> I posted a new webrev addressing the comments I have received so far:
>
> http://cr.openjdk.java.net/~mullan/jigsaw/webrevs/SecurityManager2/webrev.01/
Mostly looks good, but I'd like to see a few changes before you push.
SimpleLibrary.java
[55] Is there a reason to put signer.ser in its own subdirectory? If
not, please change that. Also, please name it "signer", without the
extension.
[522] Serialization is a pretty heavyweight (and brittle) solution
here. I can live with it for now, but in a future changeset please
modify this code to store and load CodeSigners using the existing
conventions for metadata files.
[540] Casts should not be separated from the expression being cast,
i.e., (CodeSigner)ois.readObject(), not (CodeSigner) ois.readObject() .
Loader.java
The codeSourceForName map is awkward. Please add a codeSource property
to java.lang.reflect.Module instead. That will obviate the need for
this map and also make the CodeSource available for other uses.
[196] If there's no CodeSigner then shouldn't you pass null as the last
argument to defineModule, rather than new CodeSource(null, null)?
ModuleFileFormat.java
[831] Knowledge of the format of the signer file should be encapsulated
in one place. When installing a module file the SimpleLibrary.install
method already gets the signers, by invoking Reader.verifySignature,
so it'd be better to store the signers at that point rather than in the
module-file reader. Moving this code now will also make it possible to
eliminate serialization later.
- Mark
More information about the jigsaw-dev
mailing list