Defensively catch and log pointcut parsing exceptions

Closes gh-32838
See gh-32793
This commit is contained in:
Juergen Hoeller 2024-05-17 12:27:59 +02:00
parent 4d633c2ea8
commit 617833bec9
6 changed files with 29 additions and 14 deletions

View File

@ -41,6 +41,7 @@ import org.aspectj.weaver.tools.PointcutParameter;
import org.aspectj.weaver.tools.PointcutParser; import org.aspectj.weaver.tools.PointcutParser;
import org.aspectj.weaver.tools.PointcutPrimitive; import org.aspectj.weaver.tools.PointcutPrimitive;
import org.aspectj.weaver.tools.ShadowMatch; import org.aspectj.weaver.tools.ShadowMatch;
import org.aspectj.weaver.tools.UnsupportedPointcutPrimitiveException;
import org.springframework.aop.ClassFilter; import org.springframework.aop.ClassFilter;
import org.springframework.aop.IntroductionAwareMethodMatcher; import org.springframework.aop.IntroductionAwareMethodMatcher;
@ -115,6 +116,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
@Nullable @Nullable
private transient PointcutExpression pointcutExpression; private transient PointcutExpression pointcutExpression;
private transient boolean pointcutParsingFailed = false;
private transient Map<Method, ShadowMatch> shadowMatchCache = new ConcurrentHashMap<>(32); private transient Map<Method, ShadowMatch> shadowMatchCache = new ConcurrentHashMap<>(32);
@ -270,6 +273,10 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
@Override @Override
public boolean matches(Class<?> targetClass) { public boolean matches(Class<?> targetClass) {
if (this.pointcutParsingFailed) {
return false;
}
try { try {
try { try {
return obtainPointcutExpression().couldMatchJoinPointsInType(targetClass); return obtainPointcutExpression().couldMatchJoinPointsInType(targetClass);
@ -283,8 +290,11 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
} }
} }
} }
catch (IllegalArgumentException | IllegalStateException ex) { catch (IllegalArgumentException | IllegalStateException | UnsupportedPointcutPrimitiveException ex) {
throw ex; this.pointcutParsingFailed = true;
if (logger.isDebugEnabled()) {
logger.debug("Pointcut parser rejected expression [" + getExpression() + "]: " + ex);
}
} }
catch (Throwable ex) { catch (Throwable ex) {
logger.debug("PointcutExpression matching rejected target class", ex); logger.debug("PointcutExpression matching rejected target class", ex);

View File

@ -39,13 +39,13 @@ import org.springframework.beans.testfixture.beans.TestBean;
import org.springframework.beans.testfixture.beans.subpkg.DeepBean; import org.springframework.beans.testfixture.beans.subpkg.DeepBean;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
/** /**
* @author Rob Harrop * @author Rob Harrop
* @author Rod Johnson * @author Rod Johnson
* @author Chris Beams * @author Chris Beams
* @author Juergen Hoeller
* @author Yanming Zhou * @author Yanming Zhou
*/ */
class AspectJExpressionPointcutTests { class AspectJExpressionPointcutTests {
@ -243,7 +243,7 @@ class AspectJExpressionPointcutTests {
@Test @Test
void testInvalidExpression() { void testInvalidExpression() {
String expression = "execution(void org.springframework.beans.testfixture.beans.TestBean.setSomeNumber(Number) && args(Double)"; 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) { private TestBean getAdvisedProxy(String pointcutExpression, CallCountingInterceptor interceptor) {

View File

@ -22,11 +22,12 @@ import org.springframework.context.support.ClassPathXmlApplicationContext;
/** /**
* @author Adrian Colyer * @author Adrian Colyer
* @author Juergen Hoeller
*/ */
class AutoProxyWithCodeStyleAspectsTests { class AutoProxyWithCodeStyleAspectsTests {
@Test @Test
void noAutoproxyingOfAjcCompiledAspects() { void noAutoProxyingOfAjcCompiledAspects() {
new ClassPathXmlApplicationContext("org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml"); new ClassPathXmlApplicationContext("org/springframework/aop/aspectj/autoproxy/ajcAutoproxyTests.xml");
} }

View File

@ -2,16 +2,21 @@
<beans xmlns="http://www.springframework.org/schema/beans" <beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:aop="http://www.springframework.org/schema/aop" xmlns:aop="http://www.springframework.org/schema/aop"
xmlns:context="http://www.springframework.org/schema/context"
xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans-2.0.xsd xsi:schemaLocation="http://www.springframework.org/schema/beans https://www.springframework.org/schema/beans/spring-beans-2.0.xsd
http://www.springframework.org/schema/aop https://www.springframework.org/schema/aop/spring-aop-2.0.xsd"> 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">
<aop:aspectj-autoproxy/> <aop:aspectj-autoproxy/>
<bean id="myAspect" class="org.springframework.aop.aspectj.autoproxy.CodeStyleAspect" <context:spring-configured/>
factory-method="aspectOf">
<bean id="myAspect" class="org.springframework.aop.aspectj.autoproxy.CodeStyleAspect" factory-method="aspectOf">
<property name="foo" value="bar"/> <property name="foo" value="bar"/>
</bean> </bean>
<bean id="otherBean" class="java.lang.Object"/> <bean id="otherBean" class="java.lang.Object"/>
<bean id="yetAnotherBean" class="java.lang.Object"/>
</beans> </beans>

View File

@ -28,17 +28,14 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
* *
* @author Adrian Colyer * @author Adrian Colyer
* @author Chris Beams * @author Chris Beams
* @author Juergen Hoeller
*/ */
class OverloadedAdviceTests { class OverloadedAdviceTests {
@Test @Test
@SuppressWarnings("resource") @SuppressWarnings("resource")
void testExceptionOnConfigParsingWithMismatchedAdviceMethod() { void testConfigParsingWithMismatchedAdviceMethod() {
assertThatExceptionOfType(BeanCreationException.class) new ClassPathXmlApplicationContext(getClass().getSimpleName() + ".xml", getClass());
.isThrownBy(() -> new ClassPathXmlApplicationContext(getClass().getSimpleName() + ".xml", getClass()))
.havingRootCause()
.isInstanceOf(IllegalArgumentException.class)
.as("invalidAbsoluteTypeName should be detected by AJ").withMessageContaining("invalidAbsoluteTypeName");
} }
@Test @Test

View File

@ -18,4 +18,6 @@
<bean id="testBean" class="org.springframework.beans.testfixture.beans.TestBean"/> <bean id="testBean" class="org.springframework.beans.testfixture.beans.TestBean"/>
<bean id="testBean2" class="org.springframework.beans.testfixture.beans.TestBean"/>
</beans> </beans>