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