Skip to content

Commit 617833b

Browse files
committed
Defensively catch and log pointcut parsing exceptions
Closes gh-32838 See gh-32793
1 parent 4d633c2 commit 617833b

File tree

6 files changed

+29
-14
lines changed

6 files changed

+29
-14
lines changed

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

+12-2
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.aspectj.weaver.tools.PointcutParser;
4242
import org.aspectj.weaver.tools.PointcutPrimitive;
4343
import org.aspectj.weaver.tools.ShadowMatch;
44+
import org.aspectj.weaver.tools.UnsupportedPointcutPrimitiveException;
4445

4546
import org.springframework.aop.ClassFilter;
4647
import org.springframework.aop.IntroductionAwareMethodMatcher;
@@ -115,6 +116,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
115116
@Nullable
116117
private transient PointcutExpression pointcutExpression;
117118

119+
private transient boolean pointcutParsingFailed = false;
120+
118121
private transient Map<Method, ShadowMatch> shadowMatchCache = new ConcurrentHashMap<>(32);
119122

120123

@@ -270,6 +273,10 @@ public PointcutExpression getPointcutExpression() {
270273

271274
@Override
272275
public boolean matches(Class<?> targetClass) {
276+
if (this.pointcutParsingFailed) {
277+
return false;
278+
}
279+
273280
try {
274281
try {
275282
return obtainPointcutExpression().couldMatchJoinPointsInType(targetClass);
@@ -283,8 +290,11 @@ public boolean matches(Class<?> targetClass) {
283290
}
284291
}
285292
}
286-
catch (IllegalArgumentException | IllegalStateException ex) {
287-
throw ex;
293+
catch (IllegalArgumentException | IllegalStateException | UnsupportedPointcutPrimitiveException ex) {
294+
this.pointcutParsingFailed = true;
295+
if (logger.isDebugEnabled()) {
296+
logger.debug("Pointcut parser rejected expression [" + getExpression() + "]: " + ex);
297+
}
288298
}
289299
catch (Throwable ex) {
290300
logger.debug("PointcutExpression matching rejected target class", ex);

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
* @author Yanming Zhou
5050
*/
5151
class AspectJExpressionPointcutTests {
@@ -243,7 +243,7 @@ void testDynamicMatchingProxy() {
243243
@Test
244244
void testInvalidExpression() {
245245
String expression = "execution(void org.springframework.beans.testfixture.beans.TestBean.setSomeNumber(Number) && args(Double)";
246-
assertThatIllegalArgumentException().isThrownBy(() -> getPointcut(expression).getClassFilter().matches(Object.class));
246+
assertThat(getPointcut(expression).getClassFilter().matches(Object.class)).isFalse();
247247
}
248248

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

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,12 @@
2222

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

2829
@Test
29-
void noAutoproxyingOfAjcCompiledAspects() {
30+
void noAutoProxyingOfAjcCompiledAspects() {
3031
new ClassPathXmlApplicationContext("org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml");
3132
}
3233

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>

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

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)