Skip to content

Commit ce4912b

Browse files
committed
AspectJExpressionPointcut defensively handles fallback expression parsing
Issue: SPR-9335
1 parent 8e6e6c2 commit ce4912b

File tree

1 file changed

+59
-54
lines changed

1 file changed

+59
-54
lines changed

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

Lines changed: 59 additions & 54 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);
@@ -185,20 +186,13 @@ private void checkReadyToMatch() {
185186
throw new IllegalStateException("Must set property 'expression' before attempting to match");
186187
}
187188
if (this.pointcutExpression == null) {
188-
this.pointcutExpression = buildPointcutExpression();
189+
this.pointcutClassLoader = (this.beanFactory instanceof ConfigurableBeanFactory ?
190+
((ConfigurableBeanFactory) this.beanFactory).getBeanClassLoader() :
191+
ClassUtils.getDefaultClassLoader());
192+
this.pointcutExpression = buildPointcutExpression(this.pointcutClassLoader);
189193
}
190194
}
191195

192-
/**
193-
* Build the underlying AspectJ pointcut expression.
194-
*/
195-
private PointcutExpression buildPointcutExpression() {
196-
ClassLoader cl = (this.beanFactory instanceof ConfigurableBeanFactory ?
197-
((ConfigurableBeanFactory) this.beanFactory).getBeanClassLoader() :
198-
ClassUtils.getDefaultClassLoader());
199-
return buildPointcutExpression(cl);
200-
}
201-
202196
/**
203197
* Build the underlying AspectJ pointcut expression.
204198
*/
@@ -252,23 +246,22 @@ public PointcutExpression getPointcutExpression() {
252246
public boolean matches(Class<?> targetClass) {
253247
checkReadyToMatch();
254248
try {
255-
return this.pointcutExpression.couldMatchJoinPointsInType(targetClass);
256-
}
257-
catch (ReflectionWorldException rwe) {
258-
logger.debug("PointcutExpression matching rejected target class", rwe);
259249
try {
260-
// Actually this is still a "maybe" - treat the pointcut as dynamic if we don't know enough yet
261-
return getFallbackPointcutExpression(targetClass).couldMatchJoinPointsInType(targetClass);
250+
return this.pointcutExpression.couldMatchJoinPointsInType(targetClass);
262251
}
263-
catch (BCException bce) {
264-
logger.debug("Fallback PointcutExpression matching rejected target class", bce);
265-
return false;
252+
catch (ReflectionWorldException ex) {
253+
logger.debug("PointcutExpression matching rejected target class - trying fallback expression", ex);
254+
// Actually this is still a "maybe" - treat the pointcut as dynamic if we don't know enough yet
255+
PointcutExpression fallbackExpression = getFallbackPointcutExpression(targetClass);
256+
if (fallbackExpression != null) {
257+
return fallbackExpression.couldMatchJoinPointsInType(targetClass);
258+
}
266259
}
267260
}
268261
catch (BCException ex) {
269262
logger.debug("PointcutExpression matching rejected target class", ex);
270-
return false;
271263
}
264+
return false;
272265
}
273266

274267
@Override
@@ -365,12 +358,19 @@ protected String getCurrentProxiedBeanName() {
365358

366359

367360
/**
368-
* Get a new pointcut expression based on a target class's loader, rather
369-
* than the default.
361+
* Get a new pointcut expression based on a target class's loader rather than the default.
370362
*/
371363
private PointcutExpression getFallbackPointcutExpression(Class<?> targetClass) {
372-
ClassLoader classLoader = targetClass.getClassLoader();
373-
return (classLoader != null ? buildPointcutExpression(classLoader) : this.pointcutExpression);
364+
try {
365+
ClassLoader classLoader = targetClass.getClassLoader();
366+
if (classLoader != null && classLoader != this.pointcutClassLoader) {
367+
return buildPointcutExpression(classLoader);
368+
}
369+
}
370+
catch (Throwable ex) {
371+
logger.debug("Failed to create fallback PointcutExpression", ex);
372+
}
373+
return null;
374374
}
375375

376376
private RuntimeTestWalker getRuntimeTestWalker(ShadowMatch shadowMatch) {
@@ -396,46 +396,51 @@ private ShadowMatch getShadowMatch(Method targetMethod, Method originalMethod) {
396396
if (shadowMatch == null) {
397397
synchronized (this.shadowMatchCache) {
398398
// Not found - now check again with full lock...
399+
PointcutExpression fallbackExpression = null;
399400
Method methodToMatch = targetMethod;
400-
PointcutExpression fallbackPointcutExpression = null;
401-
shadowMatch = this.shadowMatchCache.get(methodToMatch);
401+
shadowMatch = this.shadowMatchCache.get(targetMethod);
402402
if (shadowMatch == null) {
403403
try {
404-
shadowMatch = this.pointcutExpression.matchesMethodExecution(targetMethod);
404+
shadowMatch = this.pointcutExpression.matchesMethodExecution(methodToMatch);
405405
}
406-
catch (ReflectionWorld.ReflectionWorldException ex) {
406+
catch (ReflectionWorldException ex) {
407407
// Failed to introspect target method, probably because it has been loaded
408-
// in a special ClassLoader. Let's try the original method instead...
408+
// in a special ClassLoader. Let's try the declaring ClassLoader instead...
409409
try {
410-
fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
411-
shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
412-
}
413-
catch (ReflectionWorld.ReflectionWorldException ex2) {
414-
if (targetMethod == originalMethod) {
415-
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
410+
fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
411+
if (fallbackExpression != null) {
412+
shadowMatch = fallbackExpression.matchesMethodExecution(methodToMatch);
416413
}
417-
else {
418-
try {
419-
shadowMatch = this.pointcutExpression.matchesMethodExecution(originalMethod);
420-
}
421-
catch (ReflectionWorld.ReflectionWorldException ex3) {
422-
// Could neither introspect the target class nor the proxy class ->
423-
// let's simply consider this method as non-matching.
424-
methodToMatch = originalMethod;
425-
fallbackPointcutExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
426-
try {
427-
shadowMatch = fallbackPointcutExpression.matchesMethodExecution(methodToMatch);
428-
}
429-
catch (ReflectionWorld.ReflectionWorldException ex4) {
430-
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
431-
}
414+
}
415+
catch (ReflectionWorldException ex2) {
416+
fallbackExpression = null;
417+
}
418+
}
419+
if (shadowMatch == null && targetMethod != originalMethod) {
420+
methodToMatch = originalMethod;
421+
try {
422+
shadowMatch = this.pointcutExpression.matchesMethodExecution(methodToMatch);
423+
}
424+
catch (ReflectionWorldException ex3) {
425+
// Could neither introspect the target class nor the proxy class ->
426+
// let's try the original method's declaring class before we give up...
427+
try {
428+
fallbackExpression = getFallbackPointcutExpression(methodToMatch.getDeclaringClass());
429+
if (fallbackExpression != null) {
430+
shadowMatch = fallbackExpression.matchesMethodExecution(methodToMatch);
432431
}
433432
}
433+
catch (ReflectionWorldException ex4) {
434+
fallbackExpression = null;
435+
}
434436
}
435437
}
436-
if (shadowMatch.maybeMatches() && fallbackPointcutExpression != null) {
438+
if (shadowMatch == null) {
439+
shadowMatch = new ShadowMatchImpl(org.aspectj.util.FuzzyBoolean.NO, null, null, null);
440+
}
441+
else if (shadowMatch.maybeMatches() && fallbackExpression != null) {
437442
shadowMatch = new DefensiveShadowMatch(shadowMatch,
438-
fallbackPointcutExpression.matchesMethodExecution(methodToMatch));
443+
fallbackExpression.matchesMethodExecution(methodToMatch));
439444
}
440445
this.shadowMatchCache.put(targetMethod, shadowMatch);
441446
}

0 commit comments

Comments
 (0)