Hi David,
On 05/15/2018 02:52 AM, David Holmes wrote:
Master webrev of all changes:
http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.full.v1/
I skimmed over reflection API changes.
In jl.Class:
3911 // returning a different class requires a security check
3912 SecurityManager sm = System.getSecurityManager();
3913 if (sm != null) {
3914 checkPackageAccess(sm,
3915 ClassLoader.getClassLoader(Reflection.getCallerClass()), true);
3916 }
...so here the "different" class is expected to be in the same package
as "this" class. Is this invariant enforced in VM so it need not be
checked here?
3871 * @apiNote The source language compiler is responsible for
deciding which classes
3872 * and interfaces are nestmates, by generating the appropriate
attributes
3873 * (JVMS 4.7.28 and 4.7.29) in the class file format (JVMS 4).
3874 * For example, the {@code javac} compiler
3875 * places a top-level class or interface into a nest with all
of its direct,
3876 * and indirect, {@linkplain #getDeclaredClasses() nested
classes and interfaces}
3877 * (JLS 8).
3878 * The top-level {@linkplain #getEnclosingClass() enclosing
class or interface}
3879 * is designated as the nest host.
Should the text warn about not relying on this implementation detail to
extract knowledge about nested/enclosing classes? Users should keep
using getDeclaredClasses()/getEnclosingClass() for that purpose as
nestmates may in future be used for other things too. OTOH, if users
want to do an equivalent of private access check (on behalf of real
caller and callee - for example in some kind of language runtime), it
would be better they use the nestmate API...
in j.l.r.AccessibleMember:
49 * <p> Java language access control prevents use of private
members *outside*
50 * *their top-level class;* package access members outside their
package; protected members
51 * outside their package or subclasses; and public members outside
their
52 * module unless they are declared in an {@link
Module#isExported(String,Module)
53 * exported} package and the user {@link Module#canRead reads}
their module.
Could this be interpreted also as "private members can only be accessed
from the top-level class"? I know that "nest" is not a Java language
term, so it can not be used in the context of Java language access
control. (If speaking of Java language, then even previous version of
this text was wrong - if it was right, then it wouldn't have to be
changed at all). So what about the following:
"Java language access control prevents use of private members outside
the set of classes composed of top-level class and its transitive
closure of nested classes".
Or would this be to lengthy and hard to understand?
in j.l.r.Reflection:
140 // At this point we know that currentClass can access
memberClass.
141
142 if (Modifier.isPublic(modifiers)) {
143 return true;
144 }
145
146 // Check for nestmate access if member is private
147 if (Modifier.isPrivate(modifiers)) {
148 // assert: isSubclassof(targetClass, memberClass)
although just in a function of explaining the following comment, I think
the correct assert is
// assert targetClass == null || isSubclassof(targetClass, memberClass)
as for static members, targetClass is null.
149 // Note: targetClass may be outside the nest, but that
is okay
150 // as long as memberClass is in the nest.
151 boolean nestmates = areNestMates(currentClass, memberClass);
152 if (nestmates) {
153 return true;
154 }
155 }
I'm wondering about the frequency of reflective accesses of various
kinds of members. I imagine that most frequent are accesses of public
members, others are less so (I'm not counting accesses made via
.setAccessible(true) modified Field/Method/Constructor objects as they
are bypassing this Reflection.verifyMemberAccess method). But among
others I imagine reflective access to private members is the least
frequent as there's no practical reason to use reflection inside and
among classes that are most intimately logically coupled and "know" each
other's internals at compile time. So it would make sense to check for
private access at the end, after protected and package-private checks
(that's how existing logic was structured).
So how about putting the nestmate access logic just before the last if
(!successSoFar) { return false; }:
// Check for nestmate access if member is private
if (!successSoFar && Modifier.isPrivate(modifiers)) {
// assert: targetClass == null || isSubclassof(targetClass,
memberClass)
// Note: targetClass may be outside the nest, but that is okay
// as long as memberClass is in the nest.
successSoFar = areNestMates(currentClass, memberClass);
}
if (!successSoFar) {
return false;
}
This would not penalize access to package-private and protected members
with areNestMates() JNI calls and maybe caching will not be needed at all.
Regards, Peter
