From ef2c140d3cbb5fe7dd53afd65c6ca061f617911a Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 17 May 2024 12:27:59 +0200 Subject: [PATCH] Defensively catch and log pointcut parsing exceptions Closes gh-32838 See gh-32793 (cherry picked from commit 617833bec97bdee369af01462b68f8878eb1b155) --- .../aop/aspectj/AspectJExpressionPointcut.java | 14 ++++++++++++-- .../aspectj/AspectJExpressionPointcutTests.java | 4 ++-- .../AutoProxyWithCodeStyleAspectsTests.java | 5 +++-- .../aop/aspectj/autoproxy/ajcAutoproxyTests.xml | 11 ++++++++--- .../aop/aspectj/OverloadedAdviceTests.java | 9 +++------ .../aop/aspectj/OverloadedAdviceTests.xml | 2 ++ 6 files changed, 30 insertions(+), 15 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 c4a3aa4f5085..895485a2d74f 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 @@ -42,6 +42,7 @@ import org.aspectj.weaver.tools.PointcutParser; import org.aspectj.weaver.tools.PointcutPrimitive; import org.aspectj.weaver.tools.ShadowMatch; +import org.aspectj.weaver.tools.UnsupportedPointcutPrimitiveException; import org.springframework.aop.ClassFilter; import org.springframework.aop.IntroductionAwareMethodMatcher; @@ -119,6 +120,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut @Nullable private transient PointcutExpression pointcutExpression; + private transient boolean pointcutParsingFailed = false; + private transient Map shadowMatchCache = new ConcurrentHashMap<>(32); @@ -274,6 +277,10 @@ public PointcutExpression getPointcutExpression() { @Override public boolean matches(Class targetClass) { + if (this.pointcutParsingFailed) { + return false; + } + try { try { return obtainPointcutExpression().couldMatchJoinPointsInType(targetClass); @@ -287,8 +294,11 @@ public boolean matches(Class targetClass) { } } } - catch (IllegalArgumentException | IllegalStateException ex) { - throw ex; + catch (IllegalArgumentException | IllegalStateException | UnsupportedPointcutPrimitiveException ex) { + this.pointcutParsingFailed = true; + if (logger.isDebugEnabled()) { + logger.debug("Pointcut parser rejected expression [" + getExpression() + "]: " + ex); + } } catch (Throwable ex) { logger.debug("PointcutExpression matching rejected target class", ex); diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java index 4602a1e8a9fe..c754f3d8a85a 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java @@ -39,13 +39,13 @@ import org.springframework.beans.testfixture.beans.subpkg.DeepBean; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** * @author Rob Harrop * @author Rod Johnson * @author Chris Beams + * @author Juergen Hoeller */ public class AspectJExpressionPointcutTests { @@ -244,7 +244,7 @@ public void testDynamicMatchingProxy() { @Test public void testInvalidExpression() { String expression = "execution(void org.springframework.beans.testfixture.beans.TestBean.setSomeNumber(Number) && args(Double)"; - assertThatIllegalArgumentException().isThrownBy(() -> getPointcut(expression).getClassFilter().matches(Object.class)); + assertThat(getPointcut(expression).getClassFilter().matches(Object.class)).isFalse(); } private TestBean getAdvisedProxy(String pointcutExpression, CallCountingInterceptor interceptor) { diff --git a/spring-aspects/src/test/java/org/springframework/aop/aspectj/autoproxy/AutoProxyWithCodeStyleAspectsTests.java b/spring-aspects/src/test/java/org/springframework/aop/aspectj/autoproxy/AutoProxyWithCodeStyleAspectsTests.java index 43947fa29c29..0e62726089d2 100644 --- a/spring-aspects/src/test/java/org/springframework/aop/aspectj/autoproxy/AutoProxyWithCodeStyleAspectsTests.java +++ b/spring-aspects/src/test/java/org/springframework/aop/aspectj/autoproxy/AutoProxyWithCodeStyleAspectsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,12 +22,13 @@ /** * @author Adrian Colyer + * @author Juergen Hoeller */ public class AutoProxyWithCodeStyleAspectsTests { @Test @SuppressWarnings("resource") - public void noAutoproxyingOfAjcCompiledAspects() { + public void noAutoProxyingOfAjcCompiledAspects() { new ClassPathXmlApplicationContext("org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml"); } diff --git a/spring-aspects/src/test/resources/org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml b/spring-aspects/src/test/resources/org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml index 6be707bf51dd..63d6e15591d4 100644 --- a/spring-aspects/src/test/resources/org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml +++ b/spring-aspects/src/test/resources/org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml @@ -2,16 +2,21 @@ + http://www.springframework.org/schema/aop https://www.springframework.org/schema/aop/spring-aop-2.0.xsd + http://www.springframework.org/schema/context https://www.springframework.org/schema/context/spring-context-2.5.xsd"> - + + + + + diff --git a/spring-context/src/test/java/org/springframework/aop/aspectj/OverloadedAdviceTests.java b/spring-context/src/test/java/org/springframework/aop/aspectj/OverloadedAdviceTests.java index 91597c9ab85c..257b6bf37580 100644 --- a/spring-context/src/test/java/org/springframework/aop/aspectj/OverloadedAdviceTests.java +++ b/spring-context/src/test/java/org/springframework/aop/aspectj/OverloadedAdviceTests.java @@ -28,17 +28,14 @@ * * @author Adrian Colyer * @author Chris Beams + * @author Juergen Hoeller */ class OverloadedAdviceTests { @Test @SuppressWarnings("resource") - void testExceptionOnConfigParsingWithMismatchedAdviceMethod() { - assertThatExceptionOfType(BeanCreationException.class) - .isThrownBy(() -> new ClassPathXmlApplicationContext(getClass().getSimpleName() + ".xml", getClass())) - .havingRootCause() - .isInstanceOf(IllegalArgumentException.class) - .as("invalidAbsoluteTypeName should be detected by AJ").withMessageContaining("invalidAbsoluteTypeName"); + void testConfigParsingWithMismatchedAdviceMethod() { + new ClassPathXmlApplicationContext(getClass().getSimpleName() + ".xml", getClass()); } @Test diff --git a/spring-context/src/test/resources/org/springframework/aop/aspectj/OverloadedAdviceTests.xml b/spring-context/src/test/resources/org/springframework/aop/aspectj/OverloadedAdviceTests.xml index df9bfadc8ebc..ae175f39a197 100644 --- a/spring-context/src/test/resources/org/springframework/aop/aspectj/OverloadedAdviceTests.xml +++ b/spring-context/src/test/resources/org/springframework/aop/aspectj/OverloadedAdviceTests.xml @@ -18,4 +18,6 @@ + + \ No newline at end of file