Hi Mandy,

Don't count me as reviewer; this is only editorial:

In MethodHandles.java:

============

165 * A caller, specified as a {@code Lookup} object, is in module {@code M1} is 166 * allowed to do deep reflection on module {@code M2} and package of the target class 167 * if and only if all of the following conditions are {@code true}:

There are two 'is' in this sentence, which makes it a bit difficult
to parse. Should the first be removed?

* A caller, specified as a {@code Lookup} object, in module {@code M1} is ...

============

169 * <li>If there is a security manager, its {@code checkPermission} method is 170 * called to check {@code ReflectPermission("suppressAccessChecks")} and
 171      * that must return normally.
 172      * <li>
173 * If there is a security manager, its {@code checkPermission} method is 174 * called to check {@code ReflectPermission("suppressAccessChecks")}.
 175      * This method must return normally.

Probably one of these two bullets should be removed: they look
the same to me.

=============

585 * A {@code Lookup} with {@link #PUBLIC} mode and a lookup class in {@code M1} 586 * can access public types in a module {@code M2} when {@code M2} is readable to 587 * {@code M1} when the type is in a package of {@code M2} that is exported to at least {@code M1}.
                     ^^^^
I believe there is "and" missing here:

  <<... when M2 is readable to M1 *and* when the type ....>>

===========

Is the specdiff report incomplete? I was hoping to see the table
added to the Lookup class level API under

 582      * <h1><a id="cross-module-lookup"></a>Cross-module lookups</h1>

but it seems to be missing from the specdiff?

BTW: is <h1> the right header? I thought <h1> was reserved for the
     class name. John Gibbon will know more ;-)

best regards,

-- daniel

On 02/07/2019 03:47, Mandy Chung wrote:


On 7/1/19 12:51 PM, Mandy Chung wrote:
This is an enhancement to |`Lookup::in`| and |`MethodHandles::privateLookupIn`| API
for cross module teleporting.  A `Lookup` object will record the previous
lookup class from which this |Lookup| object was teleported such that
the access check will use both the previous lookup class and the current
|lookup| context (current lookup class and allowed modes) to determine if
a type is accessible to this `Lookup` object or not.

In a nutshell, `T` in M2 is accessible to a `Lookup` object on `C`
(lookup class in M1) and `PLC` (previous lookup class in M0) if and only if
1. both M0 and M1 can read M2
2. T is in a package that is exported from M2 at least to both M0 and M1

Detailed specification is in Lookup class spec and `accessClass` javadoc.
The relevant spec about cross-module teleporting is in the Lookup class
spec and `Lookup::in` and `MethodHandles::privateLookupIn`.

CSR: https://bugs.openjdk.java.net/browse/JDK-8226916

webrev:
http://cr.openjdk.java.net/~mchung/jdk14/8173978/webrev.00

javadoc:
http://cr.openjdk.java.net/~mchung/jdk14/8173978/javadoc/java.base/java/lang/invoke/MethodHandles.Lookup.html

http://cr.openjdk.java.net/~mchung/jdk14/8173978/javadoc/java.base/java/lang/invoke/MethodHandles.html#privateLookupIn(java.lang.Class,java.lang.invoke.MethodHandles.Lookup)

I have yet to generate the spec diff. The tool is currently broken
due to javadoc change.  I'll try to workaround it and post the
spec diff soon.

specdiff:

http://cr.openjdk.java.net/~mchung/jdk14/8173978/specdiff/overview-summary.html

Mandy

Reply via email to