Merge pull request mybatis#1321 from harawata/1156-illegal-reflective…

…-access Avoid unnecessary `setAccessible()` calls.
cloudcao2 · Oct 14, 2018 · fd3b1fa · fd3b1fa
2 parents b2ad4ee + a9ea248
commit fd3b1fa
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 50 deletions.
diff --git a/src/main/java/org/apache/ibatis/reflection/Reflector.java b/src/main/java/org/apache/ibatis/reflection/Reflector.java
@@ -77,16 +77,7 @@ private void addDefaultConstructor(Class<?> clazz) {
     Constructor<?>[] consts = clazz.getDeclaredConstructors();
     for (Constructor<?> constructor : consts) {
       if (constructor.getParameterTypes().length == 0) {
-        if (canControlMemberAccessible()) {
-          try {
-            constructor.setAccessible(true);
-          } catch (Exception e) {
-            // Ignored. This is only a final precaution, nothing we can do.
-          }
-        }
-        if (constructor.isAccessible()) {
           this.defaultConstructor = constructor;
-        }
       }
     }
   }
@@ -250,26 +241,17 @@ private Class<?> typeToClass(Type src) {
   private void addFields(Class<?> clazz) {
     Field[] fields = clazz.getDeclaredFields();
     for (Field field : fields) {
-      if (canControlMemberAccessible()) {
-        try {
-          field.setAccessible(true);
-        } catch (Exception e) {
-          // Ignored. This is only a final precaution, nothing we can do.
+      if (!setMethods.containsKey(field.getName())) {
+        // issue #379 - removed the check for final because JDK 1.5 allows
+        // modification of final fields through reflection (JSR-133). (JGB)
+        // pr #16 - final static can only be set by the classloader
+        int modifiers = field.getModifiers();
+        if (!(Modifier.isFinal(modifiers) && Modifier.isStatic(modifiers))) {
+          addSetField(field);
         }
       }
-      if (field.isAccessible()) {
-        if (!setMethods.containsKey(field.getName())) {
-          // issue #379 - removed the check for final because JDK 1.5 allows
-          // modification of final fields through reflection (JSR-133). (JGB)
-          // pr #16 - final static can only be set by the classloader
-          int modifiers = field.getModifiers();
-          if (!(Modifier.isFinal(modifiers) && Modifier.isStatic(modifiers))) {
-            addSetField(field);
-          }
-        }
-        if (!getMethods.containsKey(field.getName())) {
-          addGetField(field);
-        }
+      if (!getMethods.containsKey(field.getName())) {
+        addGetField(field);
       }
     }
     if (clazz.getSuperclass() != null) {
@@ -335,14 +317,6 @@ private void addUniqueMethods(Map<String, Method> uniqueMethods, Method[] method
         // if it is known, then an extended class must have
         // overridden a method
         if (!uniqueMethods.containsKey(signature)) {
-          if (canControlMemberAccessible()) {
-            try {
-              currentMethod.setAccessible(true);
-            } catch (Exception e) {
-              // Ignored. This is only a final precaution, nothing we can do.
-            }
-          }
-
           uniqueMethods.put(signature, currentMethod);
         }
       }

diff --git a/src/main/java/org/apache/ibatis/reflection/factory/DefaultObjectFactory.java b/src/main/java/org/apache/ibatis/reflection/factory/DefaultObjectFactory.java
@@ -29,6 +29,7 @@
 import java.util.TreeSet;
 
 import org.apache.ibatis.reflection.ReflectionException;
+import org.apache.ibatis.reflection.Reflector;
 
 /**
  * @author Clinton Begin
@@ -60,16 +61,28 @@ private  <T> T instantiateClass(Class<T> type, List<Class<?>> constructorArgType
       Constructor<T> constructor;
       if (constructorArgTypes == null || constructorArgs == null) {
         constructor = type.getDeclaredConstructor();
-        if (!constructor.isAccessible()) {
-          constructor.setAccessible(true);
+        try {
+          return constructor.newInstance();
+        } catch (IllegalAccessException e) {
+          if (Reflector.canControlMemberAccessible()) {
+            constructor.setAccessible(true);
+            return constructor.newInstance();
+          } else {
+            throw e;
+          }
         }
-        return constructor.newInstance();
       }
       constructor = type.getDeclaredConstructor(constructorArgTypes.toArray(new Class[constructorArgTypes.size()]));
-      if (!constructor.isAccessible()) {
-        constructor.setAccessible(true);
+      try {
+        return constructor.newInstance(constructorArgs.toArray(new Object[constructorArgs.size()]));
+      } catch (IllegalAccessException e) {
+        if (Reflector.canControlMemberAccessible()) {
+          constructor.setAccessible(true);
+          return constructor.newInstance(constructorArgs.toArray(new Object[constructorArgs.size()]));
+        } else {
+          throw e;
+        }
       }
-      return constructor.newInstance(constructorArgs.toArray(new Object[constructorArgs.size()]));
     } catch (Exception e) {
       StringBuilder argTypes = new StringBuilder();
       if (constructorArgTypes != null && !constructorArgTypes.isEmpty()) {

diff --git a/src/main/java/org/apache/ibatis/reflection/invoker/GetFieldInvoker.java b/src/main/java/org/apache/ibatis/reflection/invoker/GetFieldInvoker.java
@@ -1,5 +1,5 @@
 /**
- *    Copyright 2009-2017 the original author or authors.
+ *    Copyright 2009-2018 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -18,6 +18,8 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 
+import org.apache.ibatis.reflection.Reflector;
+
 /**
  * @author Clinton Begin
  */
@@ -30,7 +32,16 @@ public GetFieldInvoker(Field field) {
 
   @Override
   public Object invoke(Object target, Object[] args) throws IllegalAccessException, InvocationTargetException {
-    return field.get(target);
+    try {
+      return field.get(target);
+    } catch (IllegalAccessException e) {
+      if (Reflector.canControlMemberAccessible()) {
+        field.setAccessible(true);
+        return field.get(target);
+      } else {
+        throw e;
+      }
+    }
   }
 
   @Override

diff --git a/src/main/java/org/apache/ibatis/reflection/invoker/MethodInvoker.java b/src/main/java/org/apache/ibatis/reflection/invoker/MethodInvoker.java
@@ -1,5 +1,5 @@
 /**
- *    Copyright 2009-2017 the original author or authors.
+ *    Copyright 2009-2018 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -18,6 +18,8 @@
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 
+import org.apache.ibatis.reflection.Reflector;
+
 /**
  * @author Clinton Begin
  */
@@ -38,7 +40,16 @@ public MethodInvoker(Method method) {
 
   @Override
   public Object invoke(Object target, Object[] args) throws IllegalAccessException, InvocationTargetException {
-    return method.invoke(target, args);
+    try {
+      return method.invoke(target, args);
+    } catch (IllegalAccessException e) {
+      if (Reflector.canControlMemberAccessible()) {
+        method.setAccessible(true);
+        return method.invoke(target, args);
+      } else {
+        throw e;
+      }
+    }
   }
 
   @Override

diff --git a/src/main/java/org/apache/ibatis/reflection/invoker/SetFieldInvoker.java b/src/main/java/org/apache/ibatis/reflection/invoker/SetFieldInvoker.java
@@ -1,5 +1,5 @@
 /**
- *    Copyright 2009-2017 the original author or authors.
+ *    Copyright 2009-2018 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -18,6 +18,8 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 
+import org.apache.ibatis.reflection.Reflector;
+
 /**
  * @author Clinton Begin
  */
@@ -30,7 +32,16 @@ public SetFieldInvoker(Field field) {
 
   @Override
   public Object invoke(Object target, Object[] args) throws IllegalAccessException, InvocationTargetException {
-    field.set(target, args[0]);
+    try {
+      field.set(target, args[0]);
+    } catch (IllegalAccessException e) {
+      if (Reflector.canControlMemberAccessible()) {
+        field.setAccessible(true);
+        field.set(target, args[0]);
+      } else {
+        throw e;
+      }
+    }
     return null;
   }
 

diff --git a/src/main/java/org/apache/ibatis/reflection/property/PropertyCopier.java b/src/main/java/org/apache/ibatis/reflection/property/PropertyCopier.java
@@ -1,5 +1,5 @@
 /**
- *    Copyright 2009-2015 the original author or authors.
+ *    Copyright 2009-2018 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -17,6 +17,8 @@
 
 import java.lang.reflect.Field;
 
+import org.apache.ibatis.reflection.Reflector;
+
 /**
  * @author Clinton Begin
  */
@@ -32,8 +34,16 @@ public static void copyBeanProperties(Class<?> type, Object sourceBean, Object d
       final Field[] fields = parent.getDeclaredFields();
       for(Field field : fields) {
         try {
-          field.setAccessible(true);
-          field.set(destinationBean, field.get(sourceBean));
+          try {
+            field.set(destinationBean, field.get(sourceBean));
+          } catch (IllegalAccessException e) {
+            if (Reflector.canControlMemberAccessible()) {
+              field.setAccessible(true);
+              field.set(destinationBean, field.get(sourceBean));
+            } else {
+              throw e;
+            }
+          }
         } catch (Exception e) {
           // Nothing useful to do, will only fail on final fields, which will be ignored.
         }