Always fall back to original method if annotation pointcut used

Prior to this commit, AspectJExpressionPointcut doesn't fall back to original method if `!@annotation()` is used, it can cause false positive result.

Fix GH-27119
This commit is contained in:
Yanming Zhou 2023-05-24 12:23:32 +08:00 committed by Juergen Hoeller
parent 16c6376b3f
commit 3a6d0c1d5b
2 changed files with 35 additions and 3 deletions

View File

@ -78,6 +78,7 @@ import org.springframework.util.StringUtils;
* @author Juergen Hoeller
* @author Ramnivas Laddad
* @author Dave Syer
* @author Yanming Zhou
* @since 2.0
*/
@SuppressWarnings("serial")
@ -470,8 +471,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
fallbackExpression = null;
}
}
if (targetMethod != originalMethod && (shadowMatch == null ||
(shadowMatch.neverMatches() && Proxy.isProxyClass(targetMethod.getDeclaringClass())))) {
if (targetMethod != originalMethod && (shadowMatch == null || shouldFallback(targetMethod))) {
// Fall back to the plain original method in case of no resolvable match or a
// negative match on a proxy class (which doesn't carry any annotations on its
// redeclared methods).
@ -513,6 +513,12 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
return shadowMatch;
}
private boolean shouldFallback(Method targetMethod) {
if (!Proxy.isProxyClass(targetMethod.getDeclaringClass())) {
return false;
}
return this.resolveExpression().contains("@annotation");
}
@Override
public boolean equals(@Nullable Object other) {

View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2023 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.
@ -50,6 +50,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
* @author Rob Harrop
* @author Rod Johnson
* @author Chris Beams
* @author Yanming Zhou
*/
public class AspectJExpressionPointcutTests {
@ -424,6 +425,19 @@ public class AspectJExpressionPointcutTests {
assertThat(ajexp.matches(BeanA.class.getMethod("getAge"), proxy.getClass())).isTrue();
}
@Test
public void testNotAnnotationOnCglibProxyMethod() throws Exception {
String expression = "!@annotation(test.annotation.transaction.Tx)";
AspectJExpressionPointcut ajexp = new AspectJExpressionPointcut();
ajexp.setExpression(expression);
ProxyFactory factory = new ProxyFactory(new BeanA());
factory.setProxyTargetClass(true);
BeanA proxy = (BeanA) factory.getProxy();
assertThat(ajexp.matches(BeanA.class.getMethod("getAge"), proxy.getClass())).isFalse();
}
@Test
public void testAnnotationOnDynamicProxyMethod() throws Exception {
String expression = "@annotation(test.annotation.transaction.Tx)";
@ -436,6 +450,18 @@ public class AspectJExpressionPointcutTests {
assertThat(ajexp.matches(IBeanA.class.getMethod("getAge"), proxy.getClass())).isTrue();
}
@Test
public void testNotAnnotationOnDynamicProxyMethod() throws Exception {
String expression = "!@annotation(test.annotation.transaction.Tx)";
AspectJExpressionPointcut ajexp = new AspectJExpressionPointcut();
ajexp.setExpression(expression);
ProxyFactory factory = new ProxyFactory(new BeanA());
factory.setProxyTargetClass(false);
IBeanA proxy = (IBeanA) factory.getProxy();
assertThat(ajexp.matches(IBeanA.class.getMethod("getAge"), proxy.getClass())).isFalse();
}
@Test
public void testAnnotationOnMethodWithWildcard() throws Exception {
String expression = "execution(@(test.annotation..*) * *(..))";