Skip to content

Commit ed0c2ff

Browse files
jhoellerjhoeller
committed
Restore ability to return original method for proxy-derived method
Closes gh-32365
1 parent e668e77 commit ed0c2ff

File tree

3 files changed

+91
-37
lines changed

3 files changed

+91
-37
lines changed

‎spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -186,10 +186,10 @@ public static boolean isFinalizeMethod(@Nullable Method method) {
186186
* this method resolves bridge methods in order to retrieve attributes from
187187
* the <i>original</i> method definition.
188188
* @param method the method to be invoked, which may come from an interface
189-
* @param targetClass the target class for the current invocation.
190-
* May be {@code null} or may not even implement the method.
189+
* @param targetClass the target class for the current invocation
190+
* (can be {@code null} or may not even implement the method)
191191
* @return the specific target method, or the original method if the
192-
* {@code targetClass} doesn't implement it or is {@code null}
192+
* {@code targetClass} does not implement it
193193
* @see org.springframework.util.ClassUtils#getMostSpecificMethod
194194
*/
195195
public static Method getMostSpecificMethod(Method method, @Nullable Class<?> targetClass) {

‎spring-aop/src/test/java/org/springframework/aop/support/AopUtilsTests.java

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,29 +17,35 @@
1717
package org.springframework.aop.support;
1818

1919
import java.lang.reflect.Method;
20+
import java.util.List;
2021

2122
import org.junit.jupiter.api.Test;
2223

2324
import org.springframework.aop.ClassFilter;
2425
import org.springframework.aop.MethodMatcher;
2526
import org.springframework.aop.Pointcut;
27+
import org.springframework.aop.framework.ProxyFactory;
2628
import org.springframework.aop.interceptor.ExposeInvocationInterceptor;
2729
import org.springframework.aop.target.EmptyTargetSource;
2830
import org.springframework.aop.testfixture.interceptor.NopInterceptor;
2931
import org.springframework.beans.testfixture.beans.TestBean;
32+
import org.springframework.core.ResolvableType;
3033
import org.springframework.core.testfixture.io.SerializationTestUtils;
3134
import org.springframework.lang.Nullable;
35+
import org.springframework.util.ReflectionUtils;
3236

3337
import static org.assertj.core.api.Assertions.assertThat;
3438

3539
/**
3640
* @author Rod Johnson
3741
* @author Chris Beams
42+
* @author Sebastien Deleuze
43+
* @author Juergen Hoeller
3844
*/
39-
public class AopUtilsTests {
45+
class AopUtilsTests {
4046

4147
@Test
42-
public void testPointcutCanNeverApply() {
48+
void testPointcutCanNeverApply() {
4349
class TestPointcut extends StaticMethodMatcherPointcut {
4450
@Override
4551
public boolean matches(Method method, @Nullable Class<?> clazzy) {
@@ -52,13 +58,13 @@ public boolean matches(Method method, @Nullable Class<?> clazzy) {
5258
}
5359

5460
@Test
55-
public void testPointcutAlwaysApplies() {
61+
void testPointcutAlwaysApplies() {
5662
assertThat(AopUtils.canApply(new DefaultPointcutAdvisor(new NopInterceptor()), Object.class)).isTrue();
5763
assertThat(AopUtils.canApply(new DefaultPointcutAdvisor(new NopInterceptor()), TestBean.class)).isTrue();
5864
}
5965

6066
@Test
61-
public void testPointcutAppliesToOneMethodOnObject() {
67+
void testPointcutAppliesToOneMethodOnObject() {
6268
class TestPointcut extends StaticMethodMatcherPointcut {
6369
@Override
6470
public boolean matches(Method method, @Nullable Class<?> clazz) {
@@ -78,7 +84,7 @@ public boolean matches(Method method, @Nullable Class<?> clazz) {
7884
* that's subverted the singleton construction limitation.
7985
*/
8086
@Test
81-
public void testCanonicalFrameworkClassesStillCanonicalOnDeserialization() throws Exception {
87+
void testCanonicalFrameworkClassesStillCanonicalOnDeserialization() throws Exception {
8288
assertThat(SerializationTestUtils.serializeAndDeserialize(MethodMatcher.TRUE)).isSameAs(MethodMatcher.TRUE);
8389
assertThat(SerializationTestUtils.serializeAndDeserialize(ClassFilter.TRUE)).isSameAs(ClassFilter.TRUE);
8490
assertThat(SerializationTestUtils.serializeAndDeserialize(Pointcut.TRUE)).isSameAs(Pointcut.TRUE);
@@ -88,4 +94,45 @@ public void testCanonicalFrameworkClassesStillCanonicalOnDeserialization() throw
8894
assertThat(SerializationTestUtils.serializeAndDeserialize(ExposeInvocationInterceptor.INSTANCE)).isSameAs(ExposeInvocationInterceptor.INSTANCE);
8995
}
9096

97+
@Test
98+
void testInvokeJoinpointUsingReflection() throws Throwable {
99+
String name = "foo";
100+
TestBean testBean = new TestBean(name);
101+
Method method = ReflectionUtils.findMethod(TestBean.class, "getName");
102+
Object result = AopUtils.invokeJoinpointUsingReflection(testBean, method, new Object[0]);
103+
assertThat(result).isEqualTo(name);
104+
}
105+
106+
@Test // gh-32365
107+
void mostSpecificMethodBetweenJdkProxyAndTarget() throws Exception {
108+
Class<?> proxyClass = new ProxyFactory(new WithInterface()).getProxy(getClass().getClassLoader()).getClass();
109+
Method specificMethod = AopUtils.getMostSpecificMethod(proxyClass.getMethod("handle", List.class), WithInterface.class);
110+
assertThat(ResolvableType.forMethodParameter(specificMethod, 0).getGeneric().toClass()).isEqualTo(String.class);
111+
}
112+
113+
@Test // gh-32365
114+
void mostSpecificMethodBetweenCglibProxyAndTarget() throws Exception {
115+
Class<?> proxyClass = new ProxyFactory(new WithoutInterface()).getProxy(getClass().getClassLoader()).getClass();
116+
Method specificMethod = AopUtils.getMostSpecificMethod(proxyClass.getMethod("handle", List.class), WithoutInterface.class);
117+
assertThat(ResolvableType.forMethodParameter(specificMethod, 0).getGeneric().toClass()).isEqualTo(String.class);
118+
}
119+
120+
121+
interface ProxyInterface {
122+
123+
void handle(List<String> list);
124+
}
125+
126+
static class WithInterface implements ProxyInterface {
127+
128+
public void handle(List<String> list) {
129+
}
130+
}
131+
132+
static class WithoutInterface {
133+
134+
public void handle(List<String> list) {
135+
}
136+
}
137+
91138
}

‎spring-core/src/main/java/org/springframework/util/ClassUtils.java

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -44,7 +44,8 @@
4444

4545
/**
4646
* Miscellaneous {@code java.lang.Class} utility methods.
47-
* Mainly for internal use within the framework.
47+
*
48+
* <p>Mainly for internal use within the framework.
4849
*
4950
* @author Juergen Hoeller
5051
* @author Keith Donald
@@ -243,7 +244,7 @@ public static ClassLoader overrideThreadContextClassLoader(@Nullable ClassLoader
243244
* style (e.g. "java.lang.Thread.State" instead of "java.lang.Thread$State").
244245
* @param name the name of the Class
245246
* @param classLoader the class loader to use
246-
* (may be {@code null}, which indicates the default class loader)
247+
* (can be {@code null}, which indicates the default class loader)
247248
* @return a class instance for the supplied name
248249
* @throws ClassNotFoundException if the class was not found
249250
* @throws LinkageError if the class file could not be loaded
@@ -314,7 +315,7 @@ public static Class<?> forName(String name, @Nullable ClassLoader classLoader)
314315
* the exceptions thrown in case of class loading failure.
315316
* @param className the name of the Class
316317
* @param classLoader the class loader to use
317-
* (may be {@code null}, which indicates the default class loader)
318+
* (can be {@code null}, which indicates the default class loader)
318319
* @return a class instance for the supplied name
319320
* @throws IllegalArgumentException if the class name was not resolvable
320321
* (that is, the class could not be found or the class file could not be loaded)
@@ -348,7 +349,7 @@ public static Class<?> resolveClassName(String className, @Nullable ClassLoader
348349
* one of its dependencies is not present or cannot be loaded.
349350
* @param className the name of the class to check
350351
* @param classLoader the class loader to use
351-
* (may be {@code null} which indicates the default class loader)
352+
* (can be {@code null} which indicates the default class loader)
352353
* @return whether the specified class is present (including all of its
353354
* superclasses and interfaces)
354355
* @throws IllegalStateException if the corresponding class is resolvable but
@@ -375,7 +376,7 @@ public static boolean isPresent(String className, @Nullable ClassLoader classLoa
375376
* Check whether the given class is visible in the given ClassLoader.
376377
* @param clazz the class to check (typically an interface)
377378
* @param classLoader the ClassLoader to check against
378-
* (may be {@code null} in which case this method will always return {@code true})
379+
* (can be {@code null} in which case this method will always return {@code true})
379380
*/
380381
public static boolean isVisible(Class<?> clazz, @Nullable ClassLoader classLoader) {
381382
if (classLoader == null) {
@@ -399,7 +400,7 @@ public static boolean isVisible(Class<?> clazz, @Nullable ClassLoader classLoade
399400
* i.e. whether it is loaded by the given ClassLoader or a parent of it.
400401
* @param clazz the class to analyze
401402
* @param classLoader the ClassLoader to potentially cache metadata in
402-
* (may be {@code null} which indicates the system class loader)
403+
* (can be {@code null} which indicates the system class loader)
403404
*/
404405
public static boolean isCacheSafe(Class<?> clazz, @Nullable ClassLoader classLoader) {
405406
Assert.notNull(clazz, "Class must not be null");
@@ -539,9 +540,10 @@ public static Class<?> resolvePrimitiveIfNecessary(Class<?> clazz) {
539540
* Check if the right-hand side type may be assigned to the left-hand side
540541
* type, assuming setting by reflection. Considers primitive wrapper
541542
* classes as assignable to the corresponding primitive types.
542-
* @param lhsType the target type
543-
* @param rhsType the value type that should be assigned to the target type
544-
* @return if the target type is assignable from the value type
543+
* @param lhsType the target type (left-hand side (LHS) type)
544+
* @param rhsType the value type (right-hand side (RHS) type) that should
545+
* be assigned to the target type
546+
* @return {@code true} if {@code rhsType} is assignable to {@code lhsType}
545547
* @see TypeUtils#isAssignable(java.lang.reflect.Type, java.lang.reflect.Type)
546548
*/
547549
public static boolean isAssignable(Class<?> lhsType, Class<?> rhsType) {
@@ -662,7 +664,7 @@ public static String classNamesToString(Class<?>... classes) {
662664
* in the given collection.
663665
* <p>Basically like {@code AbstractCollection.toString()}, but stripping
664666
* the "class "/"interface " prefix before every class name.
665-
* @param classes a Collection of Class objects (may be {@code null})
667+
* @param classes a Collection of Class objects (can be {@code null})
666668
* @return a String of form "[com.foo.Bar, com.foo.Baz]"
667669
* @see java.util.AbstractCollection#toString()
668670
*/
@@ -717,7 +719,7 @@ public static Class<?>[] getAllInterfacesForClass(Class<?> clazz) {
717719
* <p>If the class itself is an interface, it gets returned as sole interface.
718720
* @param clazz the class to analyze for interfaces
719721
* @param classLoader the ClassLoader that the interfaces need to be visible in
720-
* (may be {@code null} when accepting all declared interfaces)
722+
* (can be {@code null} when accepting all declared interfaces)
721723
* @return all interfaces that the given object implements as an array
722724
*/
723725
public static Class<?>[] getAllInterfacesForClass(Class<?> clazz, @Nullable ClassLoader classLoader) {
@@ -752,7 +754,7 @@ public static Set<Class<?>> getAllInterfacesForClassAsSet(Class<?> clazz) {
752754
* <p>If the class itself is an interface, it gets returned as sole interface.
753755
* @param clazz the class to analyze for interfaces
754756
* @param classLoader the ClassLoader that the interfaces need to be visible in
755-
* (may be {@code null} when accepting all declared interfaces)
757+
* (can be {@code null} when accepting all declared interfaces)
756758
* @return all interfaces that the given object implements as a Set
757759
*/
758760
public static Set<Class<?>> getAllInterfacesForClassAsSet(Class<?> clazz, @Nullable ClassLoader classLoader) {
@@ -866,9 +868,9 @@ public static boolean isLambdaClass(Class<?> clazz) {
866868
/**
867869
* Check whether the given object is a CGLIB proxy.
868870
* @param object the object to check
869-
* @see #isCglibProxyClass(Class)
870871
* @see org.springframework.aop.support.AopUtils#isCglibProxy(Object)
871872
* @deprecated as of 5.2, in favor of custom (possibly narrower) checks
873+
* such as for a Spring AOP proxy
872874
*/
873875
@Deprecated
874876
public static boolean isCglibProxy(Object object) {
@@ -878,8 +880,9 @@ public static boolean isCglibProxy(Object object) {
878880
/**
879881
* Check whether the specified class is a CGLIB-generated class.
880882
* @param clazz the class to check
881-
* @see #isCglibProxyClassName(String)
883+
* @see #getUserClass(Class)
882884
* @deprecated as of 5.2, in favor of custom (possibly narrower) checks
885+
* or simply a check for containing {@link #CGLIB_CLASS_SEPARATOR}
883886
*/
884887
@Deprecated
885888
public static boolean isCglibProxyClass(@Nullable Class<?> clazz) {
@@ -889,7 +892,9 @@ public static boolean isCglibProxyClass(@Nullable Class<?> clazz) {
889892
/**
890893
* Check whether the specified class name is a CGLIB-generated class.
891894
* @param className the class name to check
895+
* @see #CGLIB_CLASS_SEPARATOR
892896
* @deprecated as of 5.2, in favor of custom (possibly narrower) checks
897+
* or simply a check for containing {@link #CGLIB_CLASS_SEPARATOR}
893898
*/
894899
@Deprecated
895900
public static boolean isCglibProxyClassName(@Nullable String className) {
@@ -913,6 +918,7 @@ public static Class<?> getUserClass(Object instance) {
913918
* class, but the original class in case of a CGLIB-generated subclass.
914919
* @param clazz the class to check
915920
* @return the user-defined class
921+
* @see #CGLIB_CLASS_SEPARATOR
916922
*/
917923
public static Class<?> getUserClass(Class<?> clazz) {
918924
if (clazz.getName().contains(CGLIB_CLASS_SEPARATOR)) {
@@ -1065,7 +1071,7 @@ public static String getQualifiedMethodName(Method method) {
10651071
* fully qualified interface/class name + "." + method name.
10661072
* @param method the method
10671073
* @param clazz the clazz that the method is being invoked on
1068-
* (may be {@code null} to indicate the method's declaring class)
1074+
* (can be {@code null} to indicate the method's declaring class)
10691075
* @return the qualified name of the method
10701076
* @since 4.3.4
10711077
*/
@@ -1146,7 +1152,7 @@ public static boolean hasMethod(Class<?> clazz, String methodName, Class<?>... p
11461152
* @param clazz the clazz to analyze
11471153
* @param methodName the name of the method
11481154
* @param paramTypes the parameter types of the method
1149-
* (may be {@code null} to indicate any signature)
1155+
* (can be {@code null} to indicate any signature)
11501156
* @return the method (never {@code null})
11511157
* @throws IllegalStateException if the method has not been found
11521158
* @see Class#getMethod
@@ -1185,7 +1191,7 @@ else if (candidates.isEmpty()) {
11851191
* @param clazz the clazz to analyze
11861192
* @param methodName the name of the method
11871193
* @param paramTypes the parameter types of the method
1188-
* (may be {@code null} to indicate any signature)
1194+
* (can be {@code null} to indicate any signature)
11891195
* @return the method, or {@code null} if not found
11901196
* @see Class#getMethod
11911197
*/
@@ -1261,26 +1267,27 @@ public static boolean hasAtLeastOneMethodWithName(Class<?> clazz, String methodN
12611267
/**
12621268
* Given a method, which may come from an interface, and a target class used
12631269
* in the current reflective invocation, find the corresponding target method
1264-
* if there is one. E.g. the method may be {@code IFoo.bar()} and the
1265-
* target class may be {@code DefaultFoo}. In this case, the method may be
1270+
* if there is one &mdash; for example, the method may be {@code IFoo.bar()},
1271+
* and the target class may be {@code DefaultFoo}. In this case, the method may be
12661272
* {@code DefaultFoo.bar()}. This enables attributes on that method to be found.
12671273
* <p><b>NOTE:</b> In contrast to {@link org.springframework.aop.support.AopUtils#getMostSpecificMethod},
12681274
* this method does <i>not</i> resolve bridge methods automatically.
12691275
* Call {@link org.springframework.core.BridgeMethodResolver#findBridgedMethod}
1270-
* if bridge method resolution is desirable (e.g. for obtaining metadata from
1271-
* the original method definition).
1272-
* <p><b>NOTE:</b> Since Spring 3.1.1, if Java security settings disallow reflective
1273-
* access (e.g. calls to {@code Class#getDeclaredMethods} etc, this implementation
1274-
* will fall back to returning the originally provided method.
1276+
* if bridge method resolution is desirable &mdash; for example, to obtain
1277+
* metadata from the original method definition.
1278+
* <p><b>NOTE:</b> If Java security settings disallow reflective access &mdash;
1279+
* for example, calls to {@code Class#getDeclaredMethods}, etc. &mdash; this
1280+
* implementation will fall back to returning the originally provided method.
12751281
* @param method the method to be invoked, which may come from an interface
12761282
* @param targetClass the target class for the current invocation
1277-
* (may be {@code null} or may not even implement the method)
1283+
* (can be {@code null} or may not even implement the method)
12781284
* @return the specific target method, or the original method if the
12791285
* {@code targetClass} does not implement it
12801286
* @see #getInterfaceMethodIfPossible(Method, Class)
12811287
*/
12821288
public static Method getMostSpecificMethod(Method method, @Nullable Class<?> targetClass) {
1283-
if (targetClass != null && targetClass != method.getDeclaringClass() && isOverridable(method, targetClass)) {
1289+
if (targetClass != null && targetClass != method.getDeclaringClass() &&
1290+
(isOverridable(method, targetClass) || !method.getDeclaringClass().isAssignableFrom(targetClass))) {
12841291
try {
12851292
if (Modifier.isPublic(method.getModifiers())) {
12861293
try {

0 commit comments

Comments
 (0)