Skip to content

Commit 228a586

Browse files
committed
AspectJExpressionPointcut defensively handles fallback expression parsing
Issue: SPR-9335 (cherry picked from commit ce4912b)
1 parent b13c5b2 commit 228a586

File tree

1 file changed

+60
-55
lines changed

1 file changed

+60
-55
lines changed

spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java

Lines changed: 60 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.apache.commons.logging.LogFactory;
3030
import org.aspectj.weaver.BCException;
3131
import org.aspectj.weaver.patterns.NamePattern;
32-
import org.aspectj.weaver.reflect.ReflectionWorld;
3332
import org.aspectj.weaver.reflect.ReflectionWorld.ReflectionWorldException;
3433
import org.aspectj.weaver.reflect.ShadowMatchImpl;
3534
import org.aspectj.weaver.tools.ContextBasedMatcher;
@@ -108,6 +107,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
108107

109108
private BeanFactory beanFactory;
110109

110+
private transient ClassLoader pointcutClassLoader;
111+
111112
private transient PointcutExpression pointcutExpression;
112113

113114
private transient Map<Method, ShadowMatch> shadowMatchCache = new ConcurrentHashMap<Method, ShadowMatch>(32);
@@ -182,20 +183,13 @@ private void checkReadyToMatch() {
182183
throw new IllegalStateException("Must set property 'expression' before attempting to match");
183184
}
184185
if (this.pointcutExpression == null) {
185-
this.pointcutExpression = buildPointcutExpression();
186+
this.pointcutClassLoader = (this.beanFactory instanceof ConfigurableBeanFactory ?
187+
((ConfigurableBeanFactory) this.beanFactory).getBeanClassLoader() :
188+
ClassUtils.getDefaultClassLoader());
189+
this.pointcutExpression = buildPointcutExpression(this.pointcutClassLoader);
186190
}
187191
}
188192

189-
/**
190-
* Build the underlying AspectJ pointcut expression.
191-
*/
192-
private PointcutExpression buildPointcutExpression() {
193-
ClassLoader cl = (this.beanFactory instanceof ConfigurableBeanFactory ?
194-
((ConfigurableBeanFactory) this.beanFactory).getBeanClassLoader() :
195-
ClassUtils.getDefaultClassLoader());
196-
return buildPointcutExpression(cl);
197-
}
198-
199193
/**
200194
* Build the underlying AspectJ pointcut expression.
201195
*/
@@ -248,23 +242,22 @@ public PointcutExpression getPointcutExpression() {
248242
public boolean matches(Class<?> targetClass) {
249243
checkReadyToMatch();
250244
try {
251-
return this.pointcutExpression.couldMatchJoinPointsInType(targetClass);
252-
}
253-
catch (ReflectionWorldException rwe) {
254-
logger.debug("PointcutExpression matching rejected target class", rwe);
255245
try {
256-
// Actually this is still a "maybe" - treat the pointcut as dynamic if we don't know enough yet
257-
return getFallbackPointcutExpression(targetClass).couldMatchJoinPointsInType(targetClass);
246+
return this.pointcutExpression.couldMatchJoinPointsInType(targetClass);
258247
}
259-
catch (BCException bce) {
260-
logger.debug("Fallback PointcutExpression matching rejected target class", bce);
261-
return false;
248+
catch (ReflectionWorldException ex) {
249+
logger.debug("PointcutExpression matching rejected target class - trying fallback expression", ex);
250+
// Actually this is still a "maybe" - treat the pointcut as dynamic if we don't know enough yet
251+
PointcutExpression fallbackExpression = getFallbackPointcutExpression(targetClass);
252+
if (fallbackExpression != null) {
253+
return fallbackExpression.couldMatchJoinPointsInType(targetClass);
254+
}
262255
}
263256
}
264257
catch (BCException ex) {
265258
logger.debug("PointcutExpression matching rejected target class", ex);
266-
return false;
267259
}
260+
return false;
268261
}
269262

270263
public boolean matches(Method method, Class<?> targetClass, boolean beanHasIntroductions) {
@@ -357,12 +350,19 @@ protected String getCurrentProxiedBeanName() {
357350

358351

359352
/**
360-
* Get a new pointcut expression based on a target class's loader, rather
361-
* than the default.
353+
* Get a new pointcut expression based on a target class's loader rather than the default.
362354
*/
363355
private PointcutExpression getFallbackPointcutExpression(Class<?> targetClass) {
364-
ClassLoader classLoader = targetClass.getClassLoader();
365-
return (classLoader != null ? buildPointcutExpression(classLoader) : this.pointcutExpression);
356+
try {
357+
ClassLoader classLoader = targetClass.getClassLoader();
358+
if (classLoader != null && classLoader != this.pointcutClassLoader) {
359+
return buildPointcutExpression(classLoader);
360+
}
361+
}
362+
catch (Throwable ex) {
363+
logger.debug("Failed to create fallback PointcutExpression", ex);
364+
}
365+
return null;
366366
}
367367

368368
private RuntimeTestWalker getRuntimeTestWalker(ShadowMatch shadowMatch) {
@@ -388,46 +388,51 @@ private ShadowMatch getShadowMatch(Method targetMethod, Method originalMethod) {
388388
if (shadowMatch == null) {
389389
synchronized (this.shadowMatchCache) {
390390
// Not found - now check again with full lock...
391+
PointcutExpression fallbackExpression = null;
391392
Method methodToMatch = targetMethod;
392-
PointcutExpression fallbackPointcutExpression = null;
393-
shadowMatch = this.shadowMatchCache.get(methodToMatch);
393+
shadowMatch = this.shadowMatchCache.get(targetMethod);
394394
if (shadowMatch == null) {
395395
try {
396-
shadowMatch = this.pointcutExpression.matchesMethodExecution(targetMethod);
396+
shadowMatch = this.pointcutExpression.matchesMethodExecution(methodToMatch);
397397
}
398-
catch (ReflectionWorld.ReflectionWorldException ex) {
398+
catch (ReflectionWorldException ex) {
399399
// Failed to introspect target method, probably because it has been loaded
400-
// in a special ClassLoader. Let's try the original method instead...
400+
// in a special ClassLoader. Let's try the declaring ClassLoader instead...
401401
try {
402-
fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
403-
shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
404-
}
405-
catch (ReflectionWorld.ReflectionWorldException ex2) {
406-
if (targetMethod == originalMethod) {
407-
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
402+
fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
403+
if (fallbackExpression != null) {
404+
shadowMatch = fallbackExpression.matchesMethodExecution(methodToMatch);
408405
}
409-
else {
410-
try {
411-
shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod);
412-
}
413-
catch (ReflectionWorld.ReflectionWorldException ex3) {
414-
// Could neither introspect the target class nor the proxy class ->
415-
// let's simply consider this method as non-matching.
416-
methodToMatch = originalMethod;
417-
fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
418-
try {
419-
shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
420-
}
421-
catch (ReflectionWorld.ReflectionWorldException ex4) {
422-
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
423-
}
406+
}
407+
catch (ReflectionWorldException ex2) {
408+
fallbackExpression = null;
409+
}
410+
}
411+
if (shadowMatch == null && targetMethod != originalMethod) {
412+
methodToMatch = originalMethod;
413+
try {
414+
shadowMatch = this.pointcutExpression.matchesMethodExecution(methodToMatch);
415+
}
416+
catch (ReflectionWorldException ex3) {
417+
// Could neither introspect the target class nor the proxy class ->
418+
// let's try the original method's declaring class before we give up...
419+
try {
420+
fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
421+
if (fallbackExpression != null) {
422+
shadowMatch = fallbackExpression.matchesMethodExecution(methodToMatch);
424423
}
425424
}
425+
catch (ReflectionWorldException ex4) {
426+
fallbackExpression = null;
427+
}
426428
}
427429
}
428-
if (shadowMatch.maybeMatches() && fallbackPointcutExpression != null) {
430+
if (shadowMatch == null) {
431+
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
432+
}
433+
else if (shadowMatch.maybeMatches() && fallbackExpression != null) {
429434
shadowMatch = new DefensiveShadowMatch(shadowMatch,
430-
fallbackPointcutExpression.matchesMethodExecution(methodToMatch));
435+
fallbackExpression.matchesMethodExecution(methodToMatch));
431436
}
432437
this.shadowMatchCache.put(targetMethod, shadowMatch);
433438
}
@@ -545,7 +550,7 @@ public boolean mayNeedDynamicTest() {
545550
return false;
546551
}
547552

548-
private FuzzyBoolean contextMatch(Class targetType) {
553+
private FuzzyBoolean contextMatch(Class<?> targetType) {
549554
String advisedBeanName = getCurrentProxiedBeanName();
550555
if (advisedBeanName == null) { // no proxy creation in progress
551556
// abstain; can't return YES, since that will make pointcut with negation fail

0 commit comments

Comments
 (0)