From ed0c2ff37f13052cc6309b32dec664a50a53c0df Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 4 Mar 2024 23:32:27 +0100 Subject: [PATCH] Restore ability to return original method for proxy-derived method Closes gh-32365 --- .../springframework/aop/support/AopUtils.java | 8 +-- .../aop/support/AopUtilsTests.java | 59 ++++++++++++++++-- .../org/springframework/util/ClassUtils.java | 61 +++++++++++-------- 3 files changed, 91 insertions(+), 37 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java b/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java index e3001ec4975..aced157280d 100644 --- a/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java +++ b/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 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. @@ -186,10 +186,10 @@ public abstract class AopUtils { * this method resolves bridge methods in order to retrieve attributes from * the original method definition. * @param method the method to be invoked, which may come from an interface - * @param targetClass the target class for the current invocation. - * May be {@code null} or may not even implement the method. + * @param targetClass the target class for the current invocation + * (can be {@code null} or may not even implement the method) * @return the specific target method, or the original method if the - * {@code targetClass} doesn't implement it or is {@code null} + * {@code targetClass} does not implement it * @see org.springframework.util.ClassUtils#getMostSpecificMethod */ public static Method getMostSpecificMethod(Method method, @Nullable Class targetClass) { diff --git a/spring-aop/src/test/java/org/springframework/aop/support/AopUtilsTests.java b/spring-aop/src/test/java/org/springframework/aop/support/AopUtilsTests.java index dc5437e6cd6..8798428aff3 100644 --- a/spring-aop/src/test/java/org/springframework/aop/support/AopUtilsTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/support/AopUtilsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 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. @@ -17,29 +17,35 @@ package org.springframework.aop.support; import java.lang.reflect.Method; +import java.util.List; import org.junit.jupiter.api.Test; import org.springframework.aop.ClassFilter; import org.springframework.aop.MethodMatcher; import org.springframework.aop.Pointcut; +import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.interceptor.ExposeInvocationInterceptor; import org.springframework.aop.target.EmptyTargetSource; import org.springframework.aop.testfixture.interceptor.NopInterceptor; import org.springframework.beans.testfixture.beans.TestBean; +import org.springframework.core.ResolvableType; import org.springframework.core.testfixture.io.SerializationTestUtils; import org.springframework.lang.Nullable; +import org.springframework.util.ReflectionUtils; import static org.assertj.core.api.Assertions.assertThat; /** * @author Rod Johnson * @author Chris Beams + * @author Sebastien Deleuze + * @author Juergen Hoeller */ -public class AopUtilsTests { +class AopUtilsTests { @Test - public void testPointcutCanNeverApply() { + void testPointcutCanNeverApply() { class TestPointcut extends StaticMethodMatcherPointcut { @Override public boolean matches(Method method, @Nullable Class clazzy) { @@ -52,13 +58,13 @@ public class AopUtilsTests { } @Test - public void testPointcutAlwaysApplies() { + void testPointcutAlwaysApplies() { assertThat(AopUtils.canApply(new DefaultPointcutAdvisor(new NopInterceptor()), Object.class)).isTrue(); assertThat(AopUtils.canApply(new DefaultPointcutAdvisor(new NopInterceptor()), TestBean.class)).isTrue(); } @Test - public void testPointcutAppliesToOneMethodOnObject() { + void testPointcutAppliesToOneMethodOnObject() { class TestPointcut extends StaticMethodMatcherPointcut { @Override public boolean matches(Method method, @Nullable Class clazz) { @@ -78,7 +84,7 @@ public class AopUtilsTests { * that's subverted the singleton construction limitation. */ @Test - public void testCanonicalFrameworkClassesStillCanonicalOnDeserialization() throws Exception { + void testCanonicalFrameworkClassesStillCanonicalOnDeserialization() throws Exception { assertThat(SerializationTestUtils.serializeAndDeserialize(MethodMatcher.TRUE)).isSameAs(MethodMatcher.TRUE); assertThat(SerializationTestUtils.serializeAndDeserialize(ClassFilter.TRUE)).isSameAs(ClassFilter.TRUE); assertThat(SerializationTestUtils.serializeAndDeserialize(Pointcut.TRUE)).isSameAs(Pointcut.TRUE); @@ -88,4 +94,45 @@ public class AopUtilsTests { assertThat(SerializationTestUtils.serializeAndDeserialize(ExposeInvocationInterceptor.INSTANCE)).isSameAs(ExposeInvocationInterceptor.INSTANCE); } + @Test + void testInvokeJoinpointUsingReflection() throws Throwable { + String name = "foo"; + TestBean testBean = new TestBean(name); + Method method = ReflectionUtils.findMethod(TestBean.class, "getName"); + Object result = AopUtils.invokeJoinpointUsingReflection(testBean, method, new Object[0]); + assertThat(result).isEqualTo(name); + } + + @Test // gh-32365 + void mostSpecificMethodBetweenJdkProxyAndTarget() throws Exception { + Class proxyClass = new ProxyFactory(new WithInterface()).getProxy(getClass().getClassLoader()).getClass(); + Method specificMethod = AopUtils.getMostSpecificMethod(proxyClass.getMethod("handle", List.class), WithInterface.class); + assertThat(ResolvableType.forMethodParameter(specificMethod, 0).getGeneric().toClass()).isEqualTo(String.class); + } + + @Test // gh-32365 + void mostSpecificMethodBetweenCglibProxyAndTarget() throws Exception { + Class proxyClass = new ProxyFactory(new WithoutInterface()).getProxy(getClass().getClassLoader()).getClass(); + Method specificMethod = AopUtils.getMostSpecificMethod(proxyClass.getMethod("handle", List.class), WithoutInterface.class); + assertThat(ResolvableType.forMethodParameter(specificMethod, 0).getGeneric().toClass()).isEqualTo(String.class); + } + + + interface ProxyInterface { + + void handle(List list); + } + + static class WithInterface implements ProxyInterface { + + public void handle(List list) { + } + } + + static class WithoutInterface { + + public void handle(List list) { + } + } + } diff --git a/spring-core/src/main/java/org/springframework/util/ClassUtils.java b/spring-core/src/main/java/org/springframework/util/ClassUtils.java index 4a84db6733b..262c544472f 100644 --- a/spring-core/src/main/java/org/springframework/util/ClassUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ClassUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 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. @@ -44,7 +44,8 @@ import org.springframework.lang.Nullable; /** * Miscellaneous {@code java.lang.Class} utility methods. - * Mainly for internal use within the framework. + * + *

Mainly for internal use within the framework. * * @author Juergen Hoeller * @author Keith Donald @@ -243,7 +244,7 @@ public abstract class ClassUtils { * style (e.g. "java.lang.Thread.State" instead of "java.lang.Thread$State"). * @param name the name of the Class * @param classLoader the class loader to use - * (may be {@code null}, which indicates the default class loader) + * (can be {@code null}, which indicates the default class loader) * @return a class instance for the supplied name * @throws ClassNotFoundException if the class was not found * @throws LinkageError if the class file could not be loaded @@ -314,7 +315,7 @@ public abstract class ClassUtils { * the exceptions thrown in case of class loading failure. * @param className the name of the Class * @param classLoader the class loader to use - * (may be {@code null}, which indicates the default class loader) + * (can be {@code null}, which indicates the default class loader) * @return a class instance for the supplied name * @throws IllegalArgumentException if the class name was not resolvable * (that is, the class could not be found or the class file could not be loaded) @@ -348,7 +349,7 @@ public abstract class ClassUtils { * one of its dependencies is not present or cannot be loaded. * @param className the name of the class to check * @param classLoader the class loader to use - * (may be {@code null} which indicates the default class loader) + * (can be {@code null} which indicates the default class loader) * @return whether the specified class is present (including all of its * superclasses and interfaces) * @throws IllegalStateException if the corresponding class is resolvable but @@ -375,7 +376,7 @@ public abstract class ClassUtils { * Check whether the given class is visible in the given ClassLoader. * @param clazz the class to check (typically an interface) * @param classLoader the ClassLoader to check against - * (may be {@code null} in which case this method will always return {@code true}) + * (can be {@code null} in which case this method will always return {@code true}) */ public static boolean isVisible(Class clazz, @Nullable ClassLoader classLoader) { if (classLoader == null) { @@ -399,7 +400,7 @@ public abstract class ClassUtils { * i.e. whether it is loaded by the given ClassLoader or a parent of it. * @param clazz the class to analyze * @param classLoader the ClassLoader to potentially cache metadata in - * (may be {@code null} which indicates the system class loader) + * (can be {@code null} which indicates the system class loader) */ public static boolean isCacheSafe(Class clazz, @Nullable ClassLoader classLoader) { Assert.notNull(clazz, "Class must not be null"); @@ -539,9 +540,10 @@ public abstract class ClassUtils { * Check if the right-hand side type may be assigned to the left-hand side * type, assuming setting by reflection. Considers primitive wrapper * classes as assignable to the corresponding primitive types. - * @param lhsType the target type - * @param rhsType the value type that should be assigned to the target type - * @return if the target type is assignable from the value type + * @param lhsType the target type (left-hand side (LHS) type) + * @param rhsType the value type (right-hand side (RHS) type) that should + * be assigned to the target type + * @return {@code true} if {@code rhsType} is assignable to {@code lhsType} * @see TypeUtils#isAssignable(java.lang.reflect.Type, java.lang.reflect.Type) */ public static boolean isAssignable(Class lhsType, Class rhsType) { @@ -662,7 +664,7 @@ public abstract class ClassUtils { * in the given collection. *

Basically like {@code AbstractCollection.toString()}, but stripping * the "class "/"interface " prefix before every class name. - * @param classes a Collection of Class objects (may be {@code null}) + * @param classes a Collection of Class objects (can be {@code null}) * @return a String of form "[com.foo.Bar, com.foo.Baz]" * @see java.util.AbstractCollection#toString() */ @@ -717,7 +719,7 @@ public abstract class ClassUtils { *

If the class itself is an interface, it gets returned as sole interface. * @param clazz the class to analyze for interfaces * @param classLoader the ClassLoader that the interfaces need to be visible in - * (may be {@code null} when accepting all declared interfaces) + * (can be {@code null} when accepting all declared interfaces) * @return all interfaces that the given object implements as an array */ public static Class[] getAllInterfacesForClass(Class clazz, @Nullable ClassLoader classLoader) { @@ -752,7 +754,7 @@ public abstract class ClassUtils { *

If the class itself is an interface, it gets returned as sole interface. * @param clazz the class to analyze for interfaces * @param classLoader the ClassLoader that the interfaces need to be visible in - * (may be {@code null} when accepting all declared interfaces) + * (can be {@code null} when accepting all declared interfaces) * @return all interfaces that the given object implements as a Set */ public static Set> getAllInterfacesForClassAsSet(Class clazz, @Nullable ClassLoader classLoader) { @@ -866,9 +868,9 @@ public abstract class ClassUtils { /** * Check whether the given object is a CGLIB proxy. * @param object the object to check - * @see #isCglibProxyClass(Class) * @see org.springframework.aop.support.AopUtils#isCglibProxy(Object) * @deprecated as of 5.2, in favor of custom (possibly narrower) checks + * such as for a Spring AOP proxy */ @Deprecated public static boolean isCglibProxy(Object object) { @@ -878,8 +880,9 @@ public abstract class ClassUtils { /** * Check whether the specified class is a CGLIB-generated class. * @param clazz the class to check - * @see #isCglibProxyClassName(String) + * @see #getUserClass(Class) * @deprecated as of 5.2, in favor of custom (possibly narrower) checks + * or simply a check for containing {@link #CGLIB_CLASS_SEPARATOR} */ @Deprecated public static boolean isCglibProxyClass(@Nullable Class clazz) { @@ -889,7 +892,9 @@ public abstract class ClassUtils { /** * Check whether the specified class name is a CGLIB-generated class. * @param className the class name to check + * @see #CGLIB_CLASS_SEPARATOR * @deprecated as of 5.2, in favor of custom (possibly narrower) checks + * or simply a check for containing {@link #CGLIB_CLASS_SEPARATOR} */ @Deprecated public static boolean isCglibProxyClassName(@Nullable String className) { @@ -913,6 +918,7 @@ public abstract class ClassUtils { * class, but the original class in case of a CGLIB-generated subclass. * @param clazz the class to check * @return the user-defined class + * @see #CGLIB_CLASS_SEPARATOR */ public static Class getUserClass(Class clazz) { if (clazz.getName().contains(CGLIB_CLASS_SEPARATOR)) { @@ -1065,7 +1071,7 @@ public abstract class ClassUtils { * fully qualified interface/class name + "." + method name. * @param method the method * @param clazz the clazz that the method is being invoked on - * (may be {@code null} to indicate the method's declaring class) + * (can be {@code null} to indicate the method's declaring class) * @return the qualified name of the method * @since 4.3.4 */ @@ -1146,7 +1152,7 @@ public abstract class ClassUtils { * @param clazz the clazz to analyze * @param methodName the name of the method * @param paramTypes the parameter types of the method - * (may be {@code null} to indicate any signature) + * (can be {@code null} to indicate any signature) * @return the method (never {@code null}) * @throws IllegalStateException if the method has not been found * @see Class#getMethod @@ -1185,7 +1191,7 @@ public abstract class ClassUtils { * @param clazz the clazz to analyze * @param methodName the name of the method * @param paramTypes the parameter types of the method - * (may be {@code null} to indicate any signature) + * (can be {@code null} to indicate any signature) * @return the method, or {@code null} if not found * @see Class#getMethod */ @@ -1261,26 +1267,27 @@ public abstract class ClassUtils { /** * Given a method, which may come from an interface, and a target class used * in the current reflective invocation, find the corresponding target method - * if there is one. E.g. the method may be {@code IFoo.bar()} and the - * target class may be {@code DefaultFoo}. In this case, the method may be + * if there is one — for example, the method may be {@code IFoo.bar()}, + * and the target class may be {@code DefaultFoo}. In this case, the method may be * {@code DefaultFoo.bar()}. This enables attributes on that method to be found. *

NOTE: In contrast to {@link org.springframework.aop.support.AopUtils#getMostSpecificMethod}, * this method does not resolve bridge methods automatically. * Call {@link org.springframework.core.BridgeMethodResolver#findBridgedMethod} - * if bridge method resolution is desirable (e.g. for obtaining metadata from - * the original method definition). - *

NOTE: Since Spring 3.1.1, if Java security settings disallow reflective - * access (e.g. calls to {@code Class#getDeclaredMethods} etc, this implementation - * will fall back to returning the originally provided method. + * if bridge method resolution is desirable — for example, to obtain + * metadata from the original method definition. + *

NOTE: If Java security settings disallow reflective access — + * for example, calls to {@code Class#getDeclaredMethods}, etc. — this + * implementation will fall back to returning the originally provided method. * @param method the method to be invoked, which may come from an interface * @param targetClass the target class for the current invocation - * (may be {@code null} or may not even implement the method) + * (can be {@code null} or may not even implement the method) * @return the specific target method, or the original method if the * {@code targetClass} does not implement it * @see #getInterfaceMethodIfPossible(Method, Class) */ public static Method getMostSpecificMethod(Method method, @Nullable Class targetClass) { - if (targetClass != null && targetClass != method.getDeclaringClass() && isOverridable(method, targetClass)) { + if (targetClass != null && targetClass != method.getDeclaringClass() && + (isOverridable(method, targetClass) || !method.getDeclaringClass().isAssignableFrom(targetClass))) { try { if (Modifier.isPublic(method.getModifiers())) { try {