SPR-5749: Add defensive matching using target class loader
* Changes to ASpectJExpressionPointcut plus some tests in Spring AOP * plus some tests in groovy support
This commit is contained in:
@@ -30,6 +30,7 @@ import org.apache.commons.logging.LogFactory;
|
||||
import org.aspectj.weaver.BCException;
|
||||
import org.aspectj.weaver.patterns.NamePattern;
|
||||
import org.aspectj.weaver.reflect.ReflectionWorld;
|
||||
import org.aspectj.weaver.reflect.ReflectionWorld.ReflectionWorldException;
|
||||
import org.aspectj.weaver.reflect.ShadowMatchImpl;
|
||||
import org.aspectj.weaver.tools.ContextBasedMatcher;
|
||||
import org.aspectj.weaver.tools.FuzzyBoolean;
|
||||
@@ -41,7 +42,6 @@ import org.aspectj.weaver.tools.PointcutParameter;
|
||||
import org.aspectj.weaver.tools.PointcutParser;
|
||||
import org.aspectj.weaver.tools.PointcutPrimitive;
|
||||
import org.aspectj.weaver.tools.ShadowMatch;
|
||||
|
||||
import org.springframework.aop.ClassFilter;
|
||||
import org.springframework.aop.IntroductionAwareMethodMatcher;
|
||||
import org.springframework.aop.MethodMatcher;
|
||||
@@ -73,6 +73,7 @@ import org.springframework.util.StringUtils;
|
||||
* @author Rod Johnson
|
||||
* @author Juergen Hoeller
|
||||
* @author Ramnivas Laddad
|
||||
* @author Dave Syer
|
||||
* @since 2.0
|
||||
*/
|
||||
public class AspectJExpressionPointcut extends AbstractExpressionPointcut
|
||||
@@ -186,30 +187,40 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
|
||||
* Build the underlying AspectJ pointcut expression.
|
||||
*/
|
||||
private PointcutExpression buildPointcutExpression() {
|
||||
PointcutParser parser = initializePointcutParser();
|
||||
ClassLoader cl = (this.beanFactory instanceof ConfigurableBeanFactory ? ((ConfigurableBeanFactory) this.beanFactory)
|
||||
.getBeanClassLoader() : Thread.currentThread()
|
||||
.getContextClassLoader());
|
||||
return buildPointcutExpression(cl);
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the underlying AspectJ pointcut expression.
|
||||
*/
|
||||
private PointcutExpression buildPointcutExpression(ClassLoader classLoader) {
|
||||
PointcutParser parser = initializePointcutParser(classLoader);
|
||||
PointcutParameter[] pointcutParameters = new PointcutParameter[this.pointcutParameterNames.length];
|
||||
for (int i = 0; i < pointcutParameters.length; i++) {
|
||||
pointcutParameters[i] = parser.createPointcutParameter(
|
||||
this.pointcutParameterNames[i], this.pointcutParameterTypes[i]);
|
||||
this.pointcutParameterNames[i],
|
||||
this.pointcutParameterTypes[i]);
|
||||
}
|
||||
return parser.parsePointcutExpression(
|
||||
replaceBooleanOperators(getExpression()), this.pointcutDeclarationScope, pointcutParameters);
|
||||
replaceBooleanOperators(getExpression()),
|
||||
this.pointcutDeclarationScope, pointcutParameters);
|
||||
}
|
||||
|
||||
/**
|
||||
* Initialize the underlying AspectJ pointcut parser.
|
||||
*/
|
||||
private PointcutParser initializePointcutParser() {
|
||||
ClassLoader cl = (this.beanFactory instanceof ConfigurableBeanFactory ?
|
||||
((ConfigurableBeanFactory) this.beanFactory).getBeanClassLoader() :
|
||||
Thread.currentThread().getContextClassLoader());
|
||||
PointcutParser parser =
|
||||
PointcutParser.getPointcutParserSupportingSpecifiedPrimitivesAndUsingSpecifiedClassLoaderForResolution(
|
||||
private PointcutParser initializePointcutParser(ClassLoader cl) {
|
||||
PointcutParser parser = PointcutParser
|
||||
.getPointcutParserSupportingSpecifiedPrimitivesAndUsingSpecifiedClassLoaderForResolution(
|
||||
SUPPORTED_PRIMITIVES, cl);
|
||||
parser.registerPointcutDesignatorHandler(new BeanNamePointcutDesignatorHandler());
|
||||
return parser;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* If a pointcut expression has been specified in XML, the user cannot
|
||||
* write <code>and</code> as "&&" (though && will work).
|
||||
@@ -236,7 +247,19 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
|
||||
checkReadyToMatch();
|
||||
try {
|
||||
return this.pointcutExpression.couldMatchJoinPointsInType(targetClass);
|
||||
}
|
||||
} catch (ReflectionWorldException e) {
|
||||
logger.debug("PointcutExpression matching rejected target class", e);
|
||||
try {
|
||||
// Actually this is still a "maybe" - treat the pointcut as dynamic if we
|
||||
// don't know enough yet
|
||||
return getFallbackPointcutExpression(targetClass).couldMatchJoinPointsInType(targetClass);
|
||||
} catch (BCException ex) {
|
||||
logger.debug(
|
||||
"Fallback PointcutExpression matching rejected target class",
|
||||
ex);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
catch (BCException ex) {
|
||||
logger.debug("PointcutExpression matching rejected target class", ex);
|
||||
return false;
|
||||
@@ -308,7 +331,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
|
||||
* <p>See SPR-2979 for the original bug.
|
||||
*/
|
||||
if (pmi != null) { // there is a current invocation
|
||||
RuntimeTestWalker originalMethodResidueTest = new RuntimeTestWalker(originalShadowMatch);
|
||||
RuntimeTestWalker originalMethodResidueTest = getRuntimeTestWalker(originalShadowMatch);
|
||||
if (!originalMethodResidueTest.testThisInstanceOfResidue(thisObject.getClass())) {
|
||||
return false;
|
||||
}
|
||||
@@ -325,6 +348,16 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Get a new pointcut expression based on a target class's loader, rather
|
||||
* than the default.
|
||||
*/
|
||||
private PointcutExpression getFallbackPointcutExpression(
|
||||
Class<?> targetClass) {
|
||||
ClassLoader classLoader = targetClass.getClassLoader();
|
||||
return classLoader == null ? this.pointcutExpression : buildPointcutExpression(classLoader);
|
||||
}
|
||||
|
||||
/**
|
||||
* A match test returned maybe - if there are any subtype sensitive variables
|
||||
* involved in the test (this, target, at_this, at_target, at_annotation) then
|
||||
@@ -332,11 +365,18 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
|
||||
* runtime subtype.
|
||||
*/
|
||||
private boolean matchesIgnoringSubtypes(ShadowMatch shadowMatch) {
|
||||
return !(new RuntimeTestWalker(shadowMatch).testsSubtypeSensitiveVars());
|
||||
return !(getRuntimeTestWalker(shadowMatch).testsSubtypeSensitiveVars());
|
||||
}
|
||||
|
||||
private boolean matchesTarget(ShadowMatch shadowMatch, Class targetClass) {
|
||||
return new RuntimeTestWalker(shadowMatch).testTargetInstanceOfResidue(targetClass);
|
||||
return getRuntimeTestWalker(shadowMatch).testTargetInstanceOfResidue(targetClass);
|
||||
}
|
||||
|
||||
private RuntimeTestWalker getRuntimeTestWalker(ShadowMatch shadowMatch) {
|
||||
if (shadowMatch instanceof DefensiveShadowMatch) {
|
||||
return new RuntimeTestWalker(((DefensiveShadowMatch)shadowMatch).primary);
|
||||
}
|
||||
return new RuntimeTestWalker(shadowMatch);
|
||||
}
|
||||
|
||||
private void bindParameters(ProxyMethodInvocation invocation, JoinPointMatch jpm) {
|
||||
@@ -355,7 +395,9 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
|
||||
if (shadowMatch == null) {
|
||||
synchronized (this.shadowMatchCache) {
|
||||
// Not found - now check again with full lock...
|
||||
shadowMatch = this.shadowMatchCache.get(targetMethod);
|
||||
Method methodToMatch = targetMethod;
|
||||
PointcutExpression fallbackPointcutExpression = null;
|
||||
shadowMatch = this.shadowMatchCache.get(methodToMatch);
|
||||
if (shadowMatch == null) {
|
||||
try {
|
||||
shadowMatch = this.pointcutExpression.matchesMethodExecution(targetMethod);
|
||||
@@ -363,20 +405,35 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
|
||||
catch (ReflectionWorld.ReflectionWorldException ex) {
|
||||
// Failed to introspect target method, probably because it has been loaded
|
||||
// in a special ClassLoader. Let's try the original method instead...
|
||||
if (targetMethod == originalMethod) {
|
||||
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
|
||||
}
|
||||
else {
|
||||
try {
|
||||
shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod);
|
||||
}
|
||||
catch (ReflectionWorld.ReflectionWorldException ex2) {
|
||||
// Could neither introspect the target class nor the proxy class ->
|
||||
// let's simply consider this method as non-matching.
|
||||
try {
|
||||
fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
|
||||
shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
|
||||
} catch (ReflectionWorld.ReflectionWorldException e) {
|
||||
if (targetMethod == originalMethod) {
|
||||
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
|
||||
}
|
||||
else {
|
||||
try {
|
||||
shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod);
|
||||
}
|
||||
catch (ReflectionWorld.ReflectionWorldException ex2) {
|
||||
// Could neither introspect the target class nor the proxy class ->
|
||||
// let's simply consider this method as non-matching.
|
||||
methodToMatch = originalMethod;
|
||||
fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
|
||||
try {
|
||||
shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
|
||||
} catch (ReflectionWorld.ReflectionWorldException e2) {
|
||||
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if (shadowMatch.maybeMatches() && fallbackPointcutExpression!=null) {
|
||||
shadowMatch = new DefensiveShadowMatch(shadowMatch,
|
||||
fallbackPointcutExpression.matchesMethodExecution(methodToMatch));
|
||||
}
|
||||
this.shadowMatchCache.put(targetMethod, shadowMatch);
|
||||
}
|
||||
}
|
||||
@@ -543,4 +600,42 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
|
||||
this.shadowMatchCache = new ConcurrentHashMap<Method, ShadowMatch>(32);
|
||||
}
|
||||
|
||||
private static class DefensiveShadowMatch implements ShadowMatch {
|
||||
|
||||
private final ShadowMatch primary;
|
||||
private final ShadowMatch other;
|
||||
|
||||
public DefensiveShadowMatch(ShadowMatch primary, ShadowMatch other) {
|
||||
this.primary = primary;
|
||||
this.other = other;
|
||||
}
|
||||
|
||||
public boolean alwaysMatches() {
|
||||
return primary.alwaysMatches();
|
||||
}
|
||||
|
||||
public boolean maybeMatches() {
|
||||
return primary.maybeMatches();
|
||||
}
|
||||
|
||||
public boolean neverMatches() {
|
||||
return primary.neverMatches();
|
||||
}
|
||||
|
||||
public JoinPointMatch matchesJoinPoint(Object thisObject,
|
||||
Object targetObject, Object[] args) {
|
||||
try {
|
||||
return primary.matchesJoinPoint(thisObject, targetObject, args);
|
||||
} catch (ReflectionWorldException e) {
|
||||
return other.matchesJoinPoint(thisObject, targetObject, args);
|
||||
}
|
||||
}
|
||||
|
||||
public void setMatchingContext(MatchingContext aMatchContext) {
|
||||
primary.setMatchingContext(aMatchContext);
|
||||
other.setMatchingContext(aMatchContext);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
package org.springframework.aop.aspectj;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
import java.lang.annotation.Documented;
|
||||
import java.lang.annotation.ElementType;
|
||||
import java.lang.annotation.Inherited;
|
||||
import java.lang.annotation.Retention;
|
||||
import java.lang.annotation.RetentionPolicy;
|
||||
import java.lang.annotation.Target;
|
||||
import java.lang.reflect.Method;
|
||||
|
||||
import org.junit.Test;
|
||||
import org.springframework.aop.Advisor;
|
||||
import org.springframework.aop.MethodBeforeAdvice;
|
||||
import org.springframework.aop.ThrowsAdvice;
|
||||
import org.springframework.aop.framework.ProxyFactory;
|
||||
import org.springframework.aop.support.DefaultPointcutAdvisor;
|
||||
import org.springframework.core.OverridingClassLoader;
|
||||
|
||||
/**
|
||||
* @author Dave Syer
|
||||
*/
|
||||
public class TrickyAspectJPointcutExpressionTests {
|
||||
|
||||
@Test
|
||||
public void testManualProxyJavaWithUnconditionalPointcut() throws Exception {
|
||||
TestService target = new TestServiceImpl();
|
||||
LogUserAdvice logAdvice = new LogUserAdvice();
|
||||
testAdvice(new DefaultPointcutAdvisor(logAdvice), logAdvice, target, "TestServiceImpl");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testManualProxyJavaWithStaticPointcut() throws Exception {
|
||||
TestService target = new TestServiceImpl();
|
||||
LogUserAdvice logAdvice = new LogUserAdvice();
|
||||
AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut();
|
||||
pointcut.setExpression(String.format("execution(* %s.TestService.*(..))", getClass().getName()));
|
||||
testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, target, "TestServiceImpl");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testManualProxyJavaWithDynamicPointcut() throws Exception {
|
||||
TestService target = new TestServiceImpl();
|
||||
LogUserAdvice logAdvice = new LogUserAdvice();
|
||||
AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut();
|
||||
pointcut.setExpression(String.format("@within(%s.Log)", getClass().getName()));
|
||||
testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, target, "TestServiceImpl");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testManualProxyJavaWithDynamicPointcutAndProxyTargetClass() throws Exception {
|
||||
TestService target = new TestServiceImpl();
|
||||
LogUserAdvice logAdvice = new LogUserAdvice();
|
||||
AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut();
|
||||
pointcut.setExpression(String.format("@within(%s.Log)", getClass().getName()));
|
||||
testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, target, "TestServiceImpl", true);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testManualProxyJavaWithStaticPointcutAndTwoClassLoaders() throws Exception {
|
||||
|
||||
LogUserAdvice logAdvice = new LogUserAdvice();
|
||||
AspectJExpressionPointcut pointcut = new AspectJExpressionPointcut();
|
||||
pointcut.setExpression(String.format("execution(* %s.TestService.*(..))", getClass().getName()));
|
||||
|
||||
// Test with default class loader first...
|
||||
testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, new TestServiceImpl(), "TestServiceImpl");
|
||||
|
||||
// Then try again with a different class loader on the target...
|
||||
SimpleThrowawayClassLoader loader = new SimpleThrowawayClassLoader(new TestServiceImpl().getClass().getClassLoader());
|
||||
// Make sure the interface is loaded from the parent class loader
|
||||
loader.excludeClass(TestService.class.getName());
|
||||
loader.excludeClass(TestException.class.getName());
|
||||
TestService other = (TestService) loader.loadClass(TestServiceImpl.class.getName()).newInstance();
|
||||
testAdvice(new DefaultPointcutAdvisor(pointcut, logAdvice), logAdvice, other, "TestServiceImpl");
|
||||
|
||||
}
|
||||
|
||||
private void testAdvice(Advisor advisor, LogUserAdvice logAdvice, TestService target, String message)
|
||||
throws Exception {
|
||||
testAdvice(advisor, logAdvice, target, message, false);
|
||||
}
|
||||
|
||||
private void testAdvice(Advisor advisor, LogUserAdvice logAdvice, TestService target, String message,
|
||||
boolean proxyTargetClass) throws Exception {
|
||||
|
||||
logAdvice.reset();
|
||||
|
||||
ProxyFactory factory = new ProxyFactory(target);
|
||||
factory.setProxyTargetClass(proxyTargetClass);
|
||||
factory.addAdvisor(advisor);
|
||||
TestService bean = (TestService) factory.getProxy();
|
||||
|
||||
assertEquals(0, logAdvice.getCountThrows());
|
||||
try {
|
||||
bean.sayHello();
|
||||
fail("Expected exception");
|
||||
} catch (TestException e) {
|
||||
assertEquals(message, e.getMessage());
|
||||
}
|
||||
assertEquals(1, logAdvice.getCountThrows());
|
||||
}
|
||||
|
||||
public static class SimpleThrowawayClassLoader extends OverridingClassLoader {
|
||||
|
||||
/**
|
||||
* Create a new SimpleThrowawayClassLoader for the given class loader.
|
||||
* @param parent the ClassLoader to build a throwaway ClassLoader for
|
||||
*/
|
||||
public SimpleThrowawayClassLoader(ClassLoader parent) {
|
||||
super(parent);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
public static class TestException extends RuntimeException {
|
||||
|
||||
public TestException(String string) {
|
||||
super(string);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@Target({ ElementType.METHOD, ElementType.TYPE })
|
||||
@Retention(RetentionPolicy.RUNTIME)
|
||||
@Documented
|
||||
@Inherited
|
||||
public static @interface Log {
|
||||
}
|
||||
|
||||
public static interface TestService {
|
||||
public String sayHello();
|
||||
}
|
||||
|
||||
@Log
|
||||
public static class TestServiceImpl implements TestService{
|
||||
public String sayHello() {
|
||||
throw new TestException("TestServiceImpl");
|
||||
}
|
||||
}
|
||||
|
||||
public class LogUserAdvice implements MethodBeforeAdvice, ThrowsAdvice {
|
||||
|
||||
private int countBefore = 0;
|
||||
|
||||
private int countThrows = 0;
|
||||
|
||||
public void before(Method method, Object[] objects, Object o) throws Throwable {
|
||||
countBefore++;
|
||||
}
|
||||
|
||||
public void afterThrowing(Exception e) throws Throwable {
|
||||
countThrows++;
|
||||
throw e;
|
||||
}
|
||||
|
||||
public int getCountBefore() {
|
||||
return countBefore;
|
||||
}
|
||||
|
||||
public int getCountThrows() {
|
||||
return countThrows;
|
||||
}
|
||||
|
||||
public void reset() {
|
||||
countThrows = 0;
|
||||
countBefore = 0;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user