Jeff,
The limited doPrivilege is very useful and I'm hoping the libraries
can begin using the limited doPrivileged once this is in jdk8. It's
non-trivial stuff and I know that you're working on the
unit tests and I can look at them when they are available.
It looks fine to me and I don't see anything obviously wrong.
This extends the wrapper implementation for the combiner
and the new comments you added make it easier to understand.
I think it'd be good to extend the javadoc of package-private
AccessControlContext constructor about "wrapped" ACC
and some summary on how they will be used in determining limited
privilege scope.
I don't spot anything obviously wrong. Just a few minor comments:
AccessController.java
You fixed up a couple of @see #doPrivileged in existing
methods. Looks like L393-394, 445-446 were copied before
the change. I'm not sure how these @see tags were
decided to be included and it seems worth taking a pass
and clean that up if appropriate.
You can call java.util.Objects.requireNonNull(perms) to
replacethe null argument check (e.g. L403-405). Might
be good to move the null argument check at the beginning?
AccessControlContext.java
L214-231: I wonder if it might be better to move this
validation to AccessController.createWrapperbefore constructing
the ACC wrapper.
The comments in the combine method e.g. L618-196, 622-623
need update after the refactoring.
L740-744: FYI this can be replaced by a simple call to:
Objects.equals(this.combiner, that.combiner)
Same comment can apply to the equality check for this.permission
and this.parent.
checkPermission2 is checking the limited privilege scope.
Maybe good to rename it to something like checkLimitedPrivilegeScope
to make it explicit.
Mandy
On 5/28/2013 10:56 PM, Jeff Nisewanger wrote:
This is the implementation of the JEP 140 feature for a limited
privilege form of doPrivileged(). A test will be added in an
updated webrev within the next day.
The JEP is: http://openjdk.java.net/jeps/140
The webrev is: http://cr.openjdk.java.net/~jdn/8014097/webrev.0/
