Improves two previous commit, by performance and bug safety

Author: yasserzamani

core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java

@@ -17,8 +17,6 @@
 
 import com.opensymphony.xwork2.util.ProxyUtil;
 import ognl.DefaultMemberAccess;
-import org.apache.commons.lang3.reflect.FieldUtils;
-import org.apache.commons.lang3.reflect.MethodUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -87,7 +85,7 @@ public boolean isAccessible(Map context, Object target, Member member, String pr
             return false;
         }
 
-        if (isProxyAccess(target, member)) {
+        if (ProxyUtil.isProxyMember(member, target)) {
             LOG.warn("Access to proxy [{}] is blocked!", member);
             return false;
         }
@@ -107,23 +105,6 @@ public boolean isAccessible(Map context, Object target, Member member, String pr
         return super.isAccessible(context, target, member, propertyName) && isAcceptableProperty(propertyName);
     }
 
-    protected boolean isProxyAccess(Object target, Member member) {
-        if (!ProxyUtil.isProxy(target))
-            return false;
-        Class<?> clazz = ProxyUtil.ultimateTargetClass(target);
-        if (member instanceof Method) {
-            return null == MethodUtils.getMatchingMethod(clazz, member.getName(), ((Method) member).getParameterTypes());
-        }
-        if (member instanceof Field) {
-            return null == FieldUtils.getField(clazz, member.getName(), true);
-        }
-        if (member instanceof Constructor) {
-            return false;
-        }
-
-        return true;
-    }
-
     protected boolean checkStaticMethodAccess(Member member) {
         int modifiers = member.getModifiers();
         if (Modifier.isStatic(modifiers)) {

core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java

@@ -15,9 +15,11 @@
  */
 package com.opensymphony.xwork2.util;
 
+import org.apache.commons.lang3.reflect.ConstructorUtils;
+import org.apache.commons.lang3.reflect.FieldUtils;
 import org.apache.commons.lang3.reflect.MethodUtils;
 
-import java.lang.reflect.Proxy;
+import java.lang.reflect.*;
 
 /**
  * <code>ProxyUtil</code>
@@ -60,6 +62,18 @@ public static boolean isProxy(Object object) {
         return isSpringAopProxy(object);
     }
 
+    /**
+     * Check whether the given member is a proxy member of a proxy object.
+     * @param member the member to check
+     * @param object the object to check
+     */
+    public static boolean isProxyMember(Member member, Object object) {
+        if (!isProxy(object))
+            return false;
+
+        return isSpringProxyMember(member);
+    }
+
     /**
      * Determine the ultimate target class of the given spring bean instance, traversing
      * not only a top-level spring proxy but any number of nested spring proxies as well &mdash;
@@ -95,6 +109,27 @@ private static boolean isSpringAopProxy(Object object) {
                 || isCglibProxyClass(clazz)));
     }
 
+    /**
+     * Check whether the given member is a member of a spring proxy.
+     * @param member the member to check
+     */
+    private static boolean isSpringProxyMember(Member member) {
+        try {
+            Class<?> clazz = ClassLoaderUtil.loadClass(SPRING_ADVISED_CLASS_NAME, ProxyUtil.class);
+            if (hasMember(clazz, member))
+                return true;
+            clazz = ClassLoaderUtil.loadClass(SPRING_TARGETCLASSAWARE_CLASS_NAME, ProxyUtil.class);
+            if (hasMember(clazz, member))
+                return true;
+            clazz = ClassLoaderUtil.loadClass(SPRING_SPRINGPROXY_CLASS_NAME, ProxyUtil.class);
+            if (hasMember(clazz, member))
+                return true;
+        } catch (ClassNotFoundException ignored) {
+        }
+
+        return false;
+    }
+
     /**
      * Obtain the singleton target object behind the given spring proxy, if any.
      * @param candidate the (potential) spring proxy to check
@@ -136,4 +171,23 @@ private static boolean implementsInterface(Class<?> clazz, String ifaceClassName
             return false;
         }
     }
+
+    /**
+     * Check whether the given class has a given member.
+     * @param clazz the class to check
+     * @param member the member to check
+     */
+    private static boolean hasMember(Class<?> clazz, Member member) {
+        if (member instanceof Method) {
+            return null != MethodUtils.getMatchingMethod(clazz, member.getName(), ((Method) member).getParameterTypes());
+        }
+        if (member instanceof Field) {
+            return null != FieldUtils.getField(clazz, member.getName(), true);
+        }
+        if (member instanceof Constructor) {
+            return null != ConstructorUtils.getMatchingAccessibleConstructor(clazz, ((Constructor) member).getParameterTypes());
+        }
+
+        return false;
+    }
 }

core/src/test/java/com/opensymphony/xwork2/spring/SpringProxyUtilTest.java

@@ -82,4 +82,30 @@ public void testUltimateTargetClass() throws Exception {
         Class<?> testAspectUltimateTargetClass = ProxyUtil.ultimateTargetClass(testAspect);
         assertEquals(TestAspect.class, testAspectUltimateTargetClass);
     }
+
+    public void testIsProxyMember() throws Exception {
+        Object simpleAction = appContext.getBean("simple-action");
+        assertFalse(ProxyUtil.isProxyMember(
+                simpleAction.getClass().getMethod("setName", String.class), simpleAction));
+
+        Object proxiedAction = appContext.getBean("proxied-action");
+        assertTrue(ProxyUtil.isProxyMember(
+                proxiedAction.getClass().getMethod("setExposeProxy", boolean.class), proxiedAction));
+
+        Object autoProxiedAction = appContext.getBean("auto-proxied-action");
+        assertTrue(ProxyUtil.isProxyMember(
+                autoProxiedAction.getClass().getMethod("getTargetClass"), autoProxiedAction));
+
+        Object pointcuttedTestBean = appContext.getBean("pointcutted-test-bean");
+        assertTrue(ProxyUtil.isProxyMember(
+                pointcuttedTestBean.getClass().getMethod("getTargetSource"), pointcuttedTestBean));
+
+        Object pointcuttedTestSubBean = appContext.getBean("pointcutted-test-sub-bean");
+        assertFalse(ProxyUtil.isProxyMember(
+                pointcuttedTestSubBean.getClass().getConstructor(), pointcuttedTestSubBean));
+
+        Object testAspect = appContext.getBean("test-aspect");
+        assertFalse(ProxyUtil.isProxyMember(
+                testAspect.getClass().getMethod("setExposeProxy", boolean.class), testAspect));
+    }
 }

core/src/test/java/com/opensymphony/xwork2/spring/TestAspect.java

@@ -7,6 +7,7 @@ public class TestAspect {
 	private int count;
 	private String name;
 	private int count2;
+    private boolean exposeProxy;
 
 	String getIssueId() {
 		return issueId;
@@ -43,4 +44,8 @@ public void setCount2(int count2) {
 		log = log + "setCount2(" + count2 + ")-";
 		this.count2 = count2;
 	}
+
+    public void setExposeProxy(boolean exposeProxy) {
+        this.exposeProxy = exposeProxy;
+    }
 }