changeset 19353:21ffffec02ce

AccessibleObject::canAccess should share access cache with internal method ::checkAccess Contributed-by: peter.levart@gmail.com
author alanb
date Mon, 20 Mar 2017 17:37:59 +0000
parents 69bb5be60882
children 38a48583e0d8
files src/java.base/share/classes/java/lang/reflect/AccessibleObject.java src/java.base/share/classes/jdk/internal/reflect/Reflection.java
diffstat 2 files changed, 48 insertions(+), 40 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/lang/reflect/AccessibleObject.java	Mon Mar 20 11:59:05 2017 +0000
+++ b/src/java.base/share/classes/java/lang/reflect/AccessibleObject.java	Mon Mar 20 17:37:59 2017 +0000
@@ -473,10 +473,7 @@
         } else {
             targetClass = Modifier.isStatic(modifiers) ? null : obj.getClass();
         }
-        return Reflection.verifyMemberAccess(caller,
-                                             declaringClass,
-                                             targetClass,
-                                             modifiers);
+        return verifyAccess(caller, declaringClass, targetClass, modifiers);
     }
 
     /**
@@ -517,7 +514,7 @@
         return AnnotatedElement.super.isAnnotationPresent(annotationClass);
     }
 
-   /**
+    /**
      * @throws NullPointerException {@inheritDoc}
      * @since 1.8
      */
@@ -588,8 +585,21 @@
                            Class<?> targetClass, int modifiers)
         throws IllegalAccessException
     {
+        if (!verifyAccess(caller, memberClass, targetClass, modifiers)) {
+            IllegalAccessException e = Reflection.newIllegalAccessException(
+                caller, memberClass, targetClass, modifiers);
+            if (printStackTraceWhenAccessFails()) {
+                e.printStackTrace(System.err);
+            }
+            throw e;
+        }
+    }
+
+    final boolean verifyAccess(Class<?> caller, Class<?> memberClass,
+                               Class<?> targetClass, int modifiers)
+    {
         if (caller == memberClass) {  // quick check
-            return;             // ACCESS IS OK
+            return true;             // ACCESS IS OK
         }
         Object cache = securityCheckCache;  // read volatile
         if (targetClass != null // instance member or constructor
@@ -600,38 +610,30 @@
                 Class<?>[] cache2 = (Class<?>[]) cache;
                 if (cache2[1] == targetClass &&
                     cache2[0] == caller) {
-                    return;     // ACCESS IS OK
+                    return true;     // ACCESS IS OK
                 }
                 // (Test cache[1] first since range check for [1]
                 // subsumes range check for [0].)
             }
         } else if (cache == caller) {
             // Non-protected case (or targetClass == memberClass or static member).
-            return;             // ACCESS IS OK
+            return true;             // ACCESS IS OK
         }
 
         // If no return, fall through to the slow path.
-        slowCheckMemberAccess(caller, memberClass, targetClass, modifiers);
+        return slowVerifyAccess(caller, memberClass, targetClass, modifiers);
     }
 
     // Keep all this slow stuff out of line:
-    void slowCheckMemberAccess(Class<?> caller, Class<?> memberClass,
-                               Class<?> targetClass, int modifiers)
-        throws IllegalAccessException
+    private boolean slowVerifyAccess(Class<?> caller, Class<?> memberClass,
+                                     Class<?> targetClass, int modifiers)
     {
         if (Reflection.verifyMemberAccess(caller, memberClass, targetClass, modifiers)) {
             // access okay
             logIfExportedByBackdoor(caller, memberClass);
         } else {
             // access denied
-            IllegalAccessException e = Reflection.newIllegalAccessException(caller,
-                                                                            memberClass,
-                                                                            targetClass,
-                                                                            modifiers);
-            if (printStackTraceWhenAccessFails()) {
-                e.printStackTrace(System.err);
-            }
-            throw e;
+            return false;
         }
 
         // Success: Update the cache.
@@ -646,6 +648,7 @@
         // guarantees that the initializing stores for the cache
         // elements will occur before the volatile write.
         securityCheckCache = cache;         // write volatile
+        return true;
     }
 
     // true to print a stack trace when access fails
--- a/src/java.base/share/classes/jdk/internal/reflect/Reflection.java	Mon Mar 20 11:59:05 2017 +0000
+++ b/src/java.base/share/classes/jdk/internal/reflect/Reflection.java	Mon Mar 20 17:37:59 2017 +0000
@@ -108,29 +108,34 @@
     }
 
     /**
-     * Verify access to a member, returning {@code false} if no access
+     * Verify access to a member and return {@code true} if it is granted.
+     *
+     * @param currentClass the class performing the access
+     * @param memberClass the declaring class of the member being accessed
+     * @param targetClass the class of target object if accessing instance
+     *                    field or method;
+     *                    or the declaring class if accessing constructor;
+     *                    or null if accessing static field or method
+     * @param modifiers the member's access modifiers
+     * @return {@code true} if access to member is granted
      */
     public static boolean verifyMemberAccess(Class<?> currentClass,
                                              Class<?> memberClass,
                                              Class<?> targetClass,
                                              int modifiers)
     {
-        // Verify that currentClass can access a field, method, or
-        // constructor of memberClass, where that member's access bits are
-        // "modifiers".
-
-        boolean gotIsSameClassPackage = false;
-        boolean isSameClassPackage = false;
-
         if (currentClass == memberClass) {
             // Always succeeds
             return true;
         }
 
-        if (!verifyModuleAccess(currentClass, memberClass)) {
+        if (!verifyModuleAccess(currentClass.getModule(), memberClass)) {
             return false;
         }
 
+        boolean gotIsSameClassPackage = false;
+        boolean isSameClassPackage = false;
+
         if (!Modifier.isPublic(getClassAccessFlags(memberClass))) {
             isSameClassPackage = isSameClassPackage(currentClass, memberClass);
             gotIsSameClassPackage = true;
@@ -190,17 +195,16 @@
     }
 
     /**
-     * Returns {@code true} if memberClass's's module exports memberClass's
-     * package to currentClass's module.
+     * Returns {@code true} if memberClass's module exports memberClass's
+     * package to currentModule.
      */
-    public static boolean verifyModuleAccess(Class<?> currentClass, Class<?> memberClass) {
-        return verifyModuleAccess(currentClass.getModule(), memberClass);
-    }
-
     public static boolean verifyModuleAccess(Module currentModule, Class<?> memberClass) {
         Module memberModule = memberClass.getModule();
         if (currentModule == memberModule) {
-            return true;  // same module (named or unnamed)
+            // same module (named or unnamed) or both null if called
+            // before module system is initialized, which means we are
+            // dealing with java.base only.
+            return true;
         } else {
             String pkg = memberClass.getPackageName();
             return memberModule.isExported(pkg, currentModule);
@@ -332,10 +336,11 @@
      * Returns an IllegalAccessException with an exception message based on
      * the access that is denied.
      */
-    public static IllegalAccessException newIllegalAccessException(Class<?> currentClass,
-                                                   Class<?> memberClass,
-                                                   Object target,
-                                                   int modifiers)
+    public static IllegalAccessException newIllegalAccessException(
+        Class<?> currentClass,
+        Class<?> memberClass,
+        Class<?> targetClass,
+        int modifiers)
         throws IllegalAccessException
     {
         String currentSuffix = "";