Skip to content

Commit ef2c140

Browse files
committed
Defensively catch and log pointcut parsing exceptions
Closes gh-32838 See gh-32793 (cherry picked from commit 617833b)
1 parent de6cf84 commit ef2c140

File tree

6 files changed

+30
-15
lines changed

6 files changed

+30
-15
lines changed

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

+12-2
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.aspectj.weaver.tools.PointcutParser;
4343
import org.aspectj.weaver.tools.PointcutPrimitive;
4444
import org.aspectj.weaver.tools.ShadowMatch;
45+
import org.aspectj.weaver.tools.UnsupportedPointcutPrimitiveException;
4546

4647
import org.springframework.aop.ClassFilter;
4748
import org.springframework.aop.IntroductionAwareMethodMatcher;
@@ -119,6 +120,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
119120
@Nullable
120121
private transient PointcutExpression pointcutExpression;
121122

123+
private transient boolean pointcutParsingFailed = false;
124+
122125
private transient Map<Method, ShadowMatch> shadowMatchCache = new ConcurrentHashMap<>(32);
123126

124127

@@ -274,6 +277,10 @@ public PointcutExpression getPointcutExpression() {
274277

275278
@Override
276279
public boolean matches(Class<?> targetClass) {
280+
if (this.pointcutParsingFailed) {
281+
return false;
282+
}
283+
277284
try {
278285
try {
279286
return obtainPointcutExpression().couldMatchJoinPointsInType(targetClass);
@@ -287,8 +294,11 @@ public boolean matches(Class<?> targetClass) {
287294
}
288295
}
289296
}
290-
catch (IllegalArgumentException | IllegalStateException ex) {
291-
throw ex;
297+
catch (IllegalArgumentException | IllegalStateException | UnsupportedPointcutPrimitiveException ex) {
298+
this.pointcutParsingFailed = true;
299+
if (logger.isDebugEnabled()) {
300+
logger.debug("Pointcut parser rejected expression [" + getExpression() + "]: " + ex);
301+
}
292302
}
293303
catch (Throwable ex) {
294304
logger.debug("PointcutExpression matching rejected target class", ex);

Diff for: spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@
3939
import org.springframework.beans.testfixture.beans.subpkg.DeepBean;
4040

4141
import static org.assertj.core.api.Assertions.assertThat;
42-
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
4342
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
4443

4544
/**
4645
* @author Rob Harrop
4746
* @author Rod Johnson
4847
* @author Chris Beams
48+
* @author Juergen Hoeller
4949
*/
5050
public class AspectJExpressionPointcutTests {
5151

@@ -244,7 +244,7 @@ public void testDynamicMatchingProxy() {
244244
@Test
245245
public void testInvalidExpression() {
246246
String expression = "execution(void org.springframework.beans.testfixture.beans.TestBean.setSomeNumber(Number) && args(Double)";
247-
assertThatIllegalArgumentException().isThrownBy(() -> getPointcut(expression).getClassFilter().matches(Object.class));
247+
assertThat(getPointcut(expression).getClassFilter().matches(Object.class)).isFalse();
248248
}
249249

250250
private TestBean getAdvisedProxy(String pointcutExpression, CallCountingInterceptor interceptor) {

Diff for: spring-aspects/src/test/java/org/springframework/aop/aspectj/autoproxy/AutoProxyWithCodeStyleAspectsTests.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2015 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,12 +22,13 @@
2222

2323
/**
2424
* @author Adrian Colyer
25+
* @author Juergen Hoeller
2526
*/
2627
public class AutoProxyWithCodeStyleAspectsTests {
2728

2829
@Test
2930
@SuppressWarnings("resource")
30-
public void noAutoproxyingOfAjcCompiledAspects() {
31+
public void noAutoProxyingOfAjcCompiledAspects() {
3132
new ClassPathXmlApplicationContext("org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml");
3233
}
3334

Diff for: spring-aspects/src/test/resources/org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml

+8-3
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,21 @@
22
<beans xmlns="http://www.springframework.org/schema/beans"
33
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
44
xmlns:aop="http://www.springframework.org/schema/aop"
5+
xmlns:context="http://www.springframework.org/schema/context"
56
xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans-2.0.xsd
6-
http://www.springframework.org/schema/aop https://www.springframework.org/schema/aop/spring-aop-2.0.xsd">
7+
http://www.springframework.org/schema/aop https://www.springframework.org/schema/aop/spring-aop-2.0.xsd
8+
http://www.springframework.org/schema/context https://www.springframework.org/schema/context/spring-context-2.5.xsd">
79

810
<aop:aspectj-autoproxy/>
911

10-
<bean id="myAspect" class="org.springframework.aop.aspectj.autoproxy.CodeStyleAspect"
11-
factory-method="aspectOf">
12+
<context:spring-configured/>
13+
14+
<bean id="myAspect" class="org.springframework.aop.aspectj.autoproxy.CodeStyleAspect" factory-method="aspectOf">
1215
<property name="foo" value="bar"/>
1316
</bean>
1417

1518
<bean id="otherBean" class="java.lang.Object"/>
1619

20+
<bean id="yetAnotherBean" class="java.lang.Object"/>
21+
1722
</beans>

Diff for: spring-context/src/test/java/org/springframework/aop/aspectj/OverloadedAdviceTests.java

+3-6
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,14 @@
2828
*
2929
* @author Adrian Colyer
3030
* @author Chris Beams
31+
* @author Juergen Hoeller
3132
*/
3233
class OverloadedAdviceTests {
3334

3435
@Test
3536
@SuppressWarnings("resource")
36-
void testExceptionOnConfigParsingWithMismatchedAdviceMethod() {
37-
assertThatExceptionOfType(BeanCreationException.class)
38-
.isThrownBy(() -> new ClassPathXmlApplicationContext(getClass().getSimpleName() + ".xml", getClass()))
39-
.havingRootCause()
40-
.isInstanceOf(IllegalArgumentException.class)
41-
.as("invalidAbsoluteTypeName should be detected by AJ").withMessageContaining("invalidAbsoluteTypeName");
37+
void testConfigParsingWithMismatchedAdviceMethod() {
38+
new ClassPathXmlApplicationContext(getClass().getSimpleName() + ".xml", getClass());
4239
}
4340

4441
@Test

Diff for: spring-context/src/test/resources/org/springframework/aop/aspectj/OverloadedAdviceTests.xml

+2
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,6 @@
1818

1919
<bean id="testBean" class="org.springframework.beans.testfixture.beans.TestBean"/>
2020

21+
<bean id="testBean2" class="org.springframework.beans.testfixture.beans.TestBean"/>
22+
2123
</beans>

0 commit comments

Comments
 (0)