From ce4912b627b4ac9be2d6bff43ef2089dc5817d67 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 28 Apr 2014 21:49:36 +0200 Subject: [PATCH] AspectJExpressionPointcut defensively handles fallback expression parsing Issue: SPR-9335 --- .../aspectj/AspectJExpressionPointcut.java | 113 +++++++++--------- 1 file changed, 59 insertions(+), 54 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java index 52b652001c..0eef2b1a56 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java @@ -29,7 +29,6 @@ import org.apache.commons.logging.Log; 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; @@ -108,6 +107,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut private BeanFactory beanFactory; + private transient ClassLoader pointcutClassLoader; + private transient PointcutExpression pointcutExpression; private transient Map shadowMatchCache = new ConcurrentHashMap(32); @@ -185,20 +186,13 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut throw new IllegalStateException("Must set property 'expression' before attempting to match"); } if (this.pointcutExpression == null) { - this.pointcutExpression = buildPointcutExpression(); + this.pointcutClassLoader = (this.beanFactory instanceof ConfigurableBeanFactory ? + ((ConfigurableBeanFactory) this.beanFactory).getBeanClassLoader() : + ClassUtils.getDefaultClassLoader()); + this.pointcutExpression = buildPointcutExpression(this.pointcutClassLoader); } } - /** - * Build the underlying AspectJ pointcut expression. - */ - private PointcutExpression buildPointcutExpression() { - ClassLoader cl = (this.beanFactory instanceof ConfigurableBeanFactory ? - ((ConfigurableBeanFactory) this.beanFactory).getBeanClassLoader() : - ClassUtils.getDefaultClassLoader()); - return buildPointcutExpression(cl); - } - /** * Build the underlying AspectJ pointcut expression. */ @@ -252,23 +246,22 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut public boolean matches(Class targetClass) { checkReadyToMatch(); try { - return this.pointcutExpression.couldMatchJoinPointsInType(targetClass); - } - catch (ReflectionWorldException rwe) { - logger.debug("PointcutExpression matching rejected target class", rwe); try { - // Actually this is still a "maybe" - treat the pointcut as dynamic if we don't know enough yet - return getFallbackPointcutExpression(targetClass).couldMatchJoinPointsInType(targetClass); + return this.pointcutExpression.couldMatchJoinPointsInType(targetClass); } - catch (BCException bce) { - logger.debug("Fallback PointcutExpression matching rejected target class", bce); - return false; + catch (ReflectionWorldException ex) { + logger.debug("PointcutExpression matching rejected target class - trying fallback expression", ex); + // Actually this is still a "maybe" - treat the pointcut as dynamic if we don't know enough yet + PointcutExpression fallbackExpression = getFallbackPointcutExpression(targetClass); + if (fallbackExpression != null) { + return fallbackExpression.couldMatchJoinPointsInType(targetClass); + } } } catch (BCException ex) { logger.debug("PointcutExpression matching rejected target class", ex); - return false; } + return false; } @Override @@ -365,12 +358,19 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut /** - * Get a new pointcut expression based on a target class's loader, rather - * than the default. + * 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 ? buildPointcutExpression(classLoader) : this.pointcutExpression); + try { + ClassLoader classLoader = targetClass.getClassLoader(); + if (classLoader != null && classLoader != this.pointcutClassLoader) { + return buildPointcutExpression(classLoader); + } + } + catch (Throwable ex) { + logger.debug("Failed to create fallback PointcutExpression", ex); + } + return null; } private RuntimeTestWalker getRuntimeTestWalker(ShadowMatch shadowMatch) { @@ -396,46 +396,51 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut if (shadowMatch == null) { synchronized (this.shadowMatchCache) { // Not found - now check again with full lock... + PointcutExpression fallbackExpression = null; Method methodToMatch = targetMethod; - PointcutExpression fallbackPointcutExpression = null; - shadowMatch = this.shadowMatchCache.get(methodToMatch); + shadowMatch = this.shadowMatchCache.get(targetMethod); if (shadowMatch == null) { try { - shadowMatch = this.pointcutExpression.matchesMethodExecution(targetMethod); + shadowMatch = this.pointcutExpression.matchesMethodExecution(methodToMatch); } - catch (ReflectionWorld.ReflectionWorldException ex) { + catch (ReflectionWorldException ex) { // Failed to introspect target method, probably because it has been loaded - // in a special ClassLoader. Let's try the original method instead... + // in a special ClassLoader. Let's try the declaring ClassLoader instead... try { - fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass()); - shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch); - } - catch (ReflectionWorld.ReflectionWorldException ex2) { - if (targetMethod == originalMethod) { - shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null); + fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass()); + if (fallbackExpression != null) { + shadowMatch = fallbackExpression.matchesMethodExecution(methodToMatch); } - else { - try { - shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod); - } - catch (ReflectionWorld.ReflectionWorldException ex3) { - // 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 ex4) { - shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null); - } + } + catch (ReflectionWorldException ex2) { + fallbackExpression = null; + } + } + if (shadowMatch == null && targetMethod != originalMethod) { + methodToMatch = originalMethod; + try { + shadowMatch = this.pointcutExpression.matchesMethodExecution(methodToMatch); + } + catch (ReflectionWorldException ex3) { + // Could neither introspect the target class nor the proxy class -> + // let's try the original method's declaring class before we give up... + try { + fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass()); + if (fallbackExpression != null) { + shadowMatch = fallbackExpression.matchesMethodExecution(methodToMatch); } } + catch (ReflectionWorldException ex4) { + fallbackExpression = null; + } } } - if (shadowMatch.maybeMatches() && fallbackPointcutExpression != null) { + if (shadowMatch == null) { + shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null); + } + else if (shadowMatch.maybeMatches() && fallbackExpression != null) { shadowMatch = new DefensiveShadowMatch(shadowMatch, - fallbackPointcutExpression.matchesMethodExecution(methodToMatch)); + fallbackExpression.matchesMethodExecution(methodToMatch)); } this.shadowMatchCache.put(targetMethod, shadowMatch); }