[core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

[David Holmes]

Hi Mandy,

Thanks for taking another look at this!

On 22/05/2018 7:47 AM, mandy chung wrote:
> On 5/20/18 10:57 PM, David Holmes wrote:
>>
>>> - I suspect the @throws SecurityException in getNestMembers was 
>>> copied from getNestHost as it uses "returned class" (singular). 
>>
>> It refers to "If any returned class ..." and "that returned class". I 
>> don't see any problematic singular uses - can you elaborate please.
>>
>>> As the host and members are in the same runtime package then maybe it 
>>> can be specified in terms of the host or members package?
>>
>> I'm not sure how to accurately formulate that. The current wording was 
>> based on similar @throws in getEnclosingClass, as suggested by Mandy:
>>
>> http://mail.openjdk.java.net/pipermail/valhalla-dev/2018-March/003955.html 
>>
>>
>> and then refined a little. 
> @throws SecurityException in my version suggested to refer to "the 
> current class".    I see your version referring to "the returned class" 
> which is what Alan commented on. getNestMembers returns more than one class.

The original copied wording was simply:

!      * @throws SecurityException
!      *         If a security manager, <i>s</i>, is present and the 
caller's
!      *         class loader is not the same as or an ancestor of the class
!      *         loader for the current class and invocation of {@link
!      *         SecurityManager#checkPackageAccess s.checkPackageAccess()}
!      *         denies access to the package of the current class

http://cr.openjdk.java.net/~dholmes/8199309/webrev/src/java.base/share/classes/java/lang/Class.java.cdiff.html

then John pointed out that we don't need to do a security check if 
returning the current class. Hence it was updated to the present wording 
to exclude that case. This was discussed in the review on valhalla-dev 
and you gave your okay to it then:

http://mail.openjdk.java.net/pipermail/valhalla-dev/2018-March/003971.html

and as I responded to Alan, for getNestMembers() it doesn't say "the 
returned class" it says "any returned class" and "that returned class". 
There is no singular/plural ambiguity.

> What about:
>    @throws SecurityException if this class is not in the nest of itself, 

I think you mean if the class is in a nest consisting solely of itself?

But that seems convoluted to me regardless. The existing statements are 
extremely clear IMHO:

getNestHost: "If the returned class is not the current class ..."

getNestMembers: "If any returned class is not the current class ..."

I don't see how they can be misinterpreted. ???

> and if a security manager, <i>s</i>, is present and the caller'sclass 
> loader is not the same as or an ancestor of the nest of thisclass and 

Something not right there - you're comparing a classloader with a nest ??

> invocation of {@linkSecurityManager#checkPackageAccess 
> s.checkPackageAccess()}denies access to the package of the nest of class.

I would not want to refer to the "package of the nest".

> The above can apply to both getNestHost and getNestMembers.
> 
> The javadoc can also explicitly state that "Classes in the same nest, 
> i.e. nest host and nest members, are in the same runtime package."

How is this adjustment in getNestHost (which is the only place where we 
explain nests):

  * <p>A <em>nest</em> is a set of classes and interfaces (nestmates) that
  * form an access control context in which each nestmate has access to the
  * private members of the other nestmates.
  * The <em>nest host</em> is the class or interface designated to hold 
the list of
  * classes and interfaces that make up the nest, and to which each of the
  * other nestmates refer.
+* All nestmates are implicitly defined in the same runtime package.

?

Thanks,
David
-----

> Mandy
> 
>