Skip to content

Commit

Permalink
Defensively catch and log pointcut parsing exceptions
Browse files Browse the repository at this point in the history
Closes gh-32838
See gh-32793

(cherry picked from commit 617833b)
  • Loading branch information
jhoeller committed May 17, 2024
1 parent de6cf84 commit ef2c140
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -119,6 +120,8 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
@Nullable
private transient PointcutExpression pointcutExpression;

private transient boolean pointcutParsingFailed = false;

private transient Map<Method, ShadowMatch> shadowMatchCache = new ConcurrentHashMap<>(32);


Expand Down Expand Up @@ -274,6 +277,10 @@ public PointcutExpression getPointcutExpression() {

@Override
public boolean matches(Class<?> targetClass) {
if (this.pointcutParsingFailed) {
return false;
}

try {
try {
return obtainPointcutExpression().couldMatchJoinPointsInType(targetClass);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,21 @@
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
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
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/>

<bean id="myAspect" class="org.springframework.aop.aspectj.autoproxy.CodeStyleAspect"
factory-method="aspectOf">
<context:spring-configured/>

<bean id="myAspect" class="org.springframework.aop.aspectj.autoproxy.CodeStyleAspect" factory-method="aspectOf">
<property name="foo" value="bar"/>
</bean>

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

<bean id="yetAnotherBean" class="java.lang.Object"/>

</beans>
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,6 @@

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

<bean id="testBean2" class="org.springframework.beans.testfixture.beans.TestBean"/>

</beans>

0 comments on commit ef2c140

Please sign in to comment.