changeset 14014:9d521fd83608

AccessibleObject.setAccessible should not be final for compatibility reasons
author alanb
date Sun, 13 Sep 2015 15:35:20 +0100
parents 6b9fef847d06
children 0321cfe06fef
files src/java.base/share/classes/java/lang/reflect/AccessibleObject.java src/java.base/share/classes/java/lang/reflect/Constructor.java src/java.base/share/classes/java/lang/reflect/Field.java src/java.base/share/classes/java/lang/reflect/Method.java
diffstat 4 files changed, 74 insertions(+), 42 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/lang/reflect/AccessibleObject.java	Sun Sep 13 14:50:36 2015 +0100
+++ b/src/java.base/share/classes/java/lang/reflect/AccessibleObject.java	Sun Sep 13 15:35:20 2015 +0100
@@ -66,15 +66,9 @@
     static final private java.security.Permission ACCESS_PERMISSION =
         new ReflectPermission("suppressAccessChecks");
 
-    /**
-     * Returns the {@code Class} object representing the class or interface
-     * that declares the executable represented by this object.
-     *
-     * This is overridden by {@code Constructor}, {@code Method} and
-     * {@code Field}.
-     */
-    Class<?> getDeclaringClass() {
-        return null;
+    static void checkPermission() {
+        SecurityManager sm = System.getSecurityManager();
+        if (sm != null) sm.checkPermission(ACCESS_PERMISSION);
     }
 
 
@@ -106,19 +100,17 @@
      * @see java.lang.RuntimePermission
      */
     @CallerSensitive
-    public static void setAccessible(AccessibleObject[] array, boolean flag)
-        throws SecurityException {
-        SecurityManager sm = System.getSecurityManager();
-        if (sm != null) sm.checkPermission(ACCESS_PERMISSION);
+    public static void setAccessible(AccessibleObject[] array, boolean flag) {
+        checkPermission();
         if (flag) {
             Class<?> caller = Reflection.getCallerClass();
             array = array.clone();
             for (AccessibleObject ao : array) {
-                checkCanSetAccessible(caller, ao);
+                ao.checkCanSetAccessible(caller);
             }
         }
         for (AccessibleObject ao : array) {
-            ao.override = flag;
+            ao.setAccessible0(flag);
         }
     }
 
@@ -150,26 +142,25 @@
      * @see SecurityManager#checkPermission
      * @see java.lang.RuntimePermission
      */
-    @CallerSensitive
-    public final void setAccessible(boolean flag) throws SecurityException {
-        SecurityManager sm = System.getSecurityManager();
-        if (sm != null) sm.checkPermission(ACCESS_PERMISSION);
-        if (flag) checkCanSetAccessible(Reflection.getCallerClass(), this);
+    public void setAccessible(boolean flag) {
+        AccessibleObject.checkPermission();
+        setAccessible0(flag);
+    }
+
+    void setAccessible0(boolean flag) {
         this.override = flag;
     }
 
-    /**
-     * If the given AccessibleObject is a {@code Constructor}, {@code Method}
-     * or {@code Field} then checks that its declaring class is in a package
-     * that can be accessed by the given caller of setAccessible.
-     */
-    private static void checkCanSetAccessible(Class<?> caller,
-                                              AccessibleObject ao)
-    {
-        Class<?> declaringClass = ao.getDeclaringClass();
-        if (declaringClass == null)
-            return; // not a Constrictor, Method or Field
+   /**
+    * If the given AccessibleObject is a {@code Constructor}, {@code Method}
+    * or {@code Field} then checks that its declaring class is in a package
+    * that can be accessed by the given caller of setAccessible.
+    */
+    void checkCanSetAccessible(Class<?> caller) {
+        // do nothing, needs to be overridden by Constructor, Method, Field
+    }
 
+    void checkCanSetAccessible(Class<?> caller, Class<?> declaringClass) {
         Module callerModule = caller.getModule();
         Module declaringModule = declaringClass.getModule();
 
@@ -196,26 +187,21 @@
 
         }
 
+        // TODO: This should throw IOE for AccessibleObject and other classes
+        // that can be easily abused to break encapsulation
         if (declaringClass == Module.class) {
             int modifiers;
-            if (ao instanceof Executable) {
-                modifiers = ((Executable)ao).getModifiers();
+            if (this instanceof Executable) {
+                modifiers = ((Executable) this).getModifiers();
             } else {
-                modifiers = ((Field)ao).getModifiers();
+                modifiers = ((Field) this).getModifiers();
             }
             if (!Modifier.isPublic(modifiers)) {
                 String msg = "Cannot make a non-public member of "
-                    + Module.class + " accessible";
+                        + declaringClass + " accessible";
                 Reflection.throwInaccessibleObjectException(msg);
             }
         }
-
-        if (declaringClass == Class.class && ao instanceof Constructor) {
-            // can we change this to InaccessibleObjectException?
-            throw new SecurityException("Cannot make a java.lang.Class"
-                    + " constructor accessible");
-        }
-
     }
 
     /**
--- a/src/java.base/share/classes/java/lang/reflect/Constructor.java	Sun Sep 13 14:50:36 2015 +0100
+++ b/src/java.base/share/classes/java/lang/reflect/Constructor.java	Sun Sep 13 15:35:20 2015 +0100
@@ -159,6 +159,26 @@
     }
 
     @Override
+    @CallerSensitive
+    public void setAccessible(boolean flag) {
+        AccessibleObject.checkPermission();
+        if (flag) {
+            checkCanSetAccessible(Reflection.getCallerClass());
+            if (clazz == Class.class) {
+                // can we change this to InaccessibleObjectException?
+                throw new SecurityException("Cannot make a java.lang.Class"
+                                            + " constructor accessible");
+            }
+        }
+        setAccessible0(flag);
+    }
+
+    @Override
+    void checkCanSetAccessible(Class<?> caller) {
+        checkCanSetAccessible(caller, clazz);
+    }
+
+    @Override
     boolean hasGenericInformation() {
         return (getSignature() != null);
     }
--- a/src/java.base/share/classes/java/lang/reflect/Field.java	Sun Sep 13 14:50:36 2015 +0100
+++ b/src/java.base/share/classes/java/lang/reflect/Field.java	Sun Sep 13 15:35:20 2015 +0100
@@ -156,6 +156,19 @@
         return res;
     }
 
+    @Override
+    @CallerSensitive
+    public void setAccessible(boolean flag) {
+        AccessibleObject.checkPermission();
+        if (flag) checkCanSetAccessible(Reflection.getCallerClass());
+        setAccessible0(flag);
+    }
+
+    @Override
+    void checkCanSetAccessible(Class<?> caller) {
+        checkCanSetAccessible(caller, clazz);
+    }
+
     /**
      * Returns the {@code Class} object representing the class or interface
      * that declares the field represented by this {@code Field} object.
--- a/src/java.base/share/classes/java/lang/reflect/Method.java	Sun Sep 13 14:50:36 2015 +0100
+++ b/src/java.base/share/classes/java/lang/reflect/Method.java	Sun Sep 13 15:35:20 2015 +0100
@@ -175,6 +175,19 @@
         return res;
     }
 
+    @Override
+    @CallerSensitive
+    public void setAccessible(boolean flag) {
+        AccessibleObject.checkPermission();
+        if (flag) checkCanSetAccessible(Reflection.getCallerClass());
+        setAccessible0(flag);
+    }
+
+    @Override
+    void checkCanSetAccessible(Class<?> caller) {
+        checkCanSetAccessible(caller, clazz);
+    }
+
     /**
      * Used by Excecutable for annotation sharing.
      */