From 7987c38d5a65700501eddd1fb0cb1a288921ecd2 Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Wed, 14 Jun 2017 18:14:19 +0430 Subject: [PATCH] Improves two previous commit, by performance and bug safety --- .../xwork2/ognl/SecurityMemberAccess.java | 21 +------ .../opensymphony/xwork2/util/ProxyUtil.java | 56 ++++++++++++++++++- .../xwork2/spring/SpringProxyUtilTest.java | 26 +++++++++ .../xwork2/spring/TestAspect.java | 5 ++ 4 files changed, 87 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java index 02c53d840..cce09cbac 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -17,8 +17,6 @@ package com.opensymphony.xwork2.ognl; 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 class SecurityMemberAccess extends DefaultMemberAccess { 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 class SecurityMemberAccess extends DefaultMemberAccess { 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)) { diff --git a/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java b/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java index afda50824..83e727cd3 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java +++ b/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.*; /** * ProxyUtil @@ -60,6 +62,18 @@ public class ProxyUtil { 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 — @@ -95,6 +109,27 @@ public class ProxyUtil { || 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 @@ public class ProxyUtil { 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; + } } diff --git a/core/src/test/java/com/opensymphony/xwork2/spring/SpringProxyUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/spring/SpringProxyUtilTest.java index 106e62d4a..42aee89de 100644 --- a/core/src/test/java/com/opensymphony/xwork2/spring/SpringProxyUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/spring/SpringProxyUtilTest.java @@ -82,4 +82,30 @@ public class SpringProxyUtilTest extends XWorkTestCase { 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)); + } } diff --git a/core/src/test/java/com/opensymphony/xwork2/spring/TestAspect.java b/core/src/test/java/com/opensymphony/xwork2/spring/TestAspect.java index a428c6588..ef2ec7c27 100644 --- a/core/src/test/java/com/opensymphony/xwork2/spring/TestAspect.java +++ b/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 class TestAspect { log = log + "setCount2(" + count2 + ")-"; this.count2 = count2; } + + public void setExposeProxy(boolean exposeProxy) { + this.exposeProxy = exposeProxy; + } }