From 38a56b3fdaade936d8660472122dc814773c89bd Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Mon, 26 Aug 2024 16:07:06 +0200 Subject: [PATCH] Unwrap CGLIB proxy when invoking non-proxied methods in ReflectionTestUtils Prior to this commit, when ReflectionTestUtils was used to invoke a method on a CGLIB proxy, the invocation was always performed directly on the proxy. Consequently, if the method was not proxied/intercepted by the CGLIB proxy -- for example, if the method was final or effectively private -- the invoked method could not operate on the state of the target object or interact with other private methods in the target object. With this commit, if the supplied target object is a CGLIB proxy which does not intercept the method, the proxy will be unwrapped allowing the method to be invoked directly on the ultimate target of the proxy. Closes gh-33429 --- .../test/util/ReflectionTestUtils.java | 90 +++++++++++++------ .../test/util/ReflectionTestUtilsTests.java | 77 +++++++++++++++- .../test/util/subpackage/PersonEntity.java | 36 ++++++-- 3 files changed, 164 insertions(+), 39 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/util/ReflectionTestUtils.java b/spring-test/src/main/java/org/springframework/test/util/ReflectionTestUtils.java index e17d334022..f63738405e 100644 --- a/spring-test/src/main/java/org/springframework/test/util/ReflectionTestUtils.java +++ b/spring-test/src/main/java/org/springframework/test/util/ReflectionTestUtils.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. @@ -22,6 +22,7 @@ import java.lang.reflect.Method; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.aop.support.AopUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -289,22 +290,14 @@ public abstract class ReflectionTestUtils { /** * Invoke the setter method with the given {@code name} on the supplied * target object with the supplied {@code value}. - *

This method traverses the class hierarchy in search of the desired - * method. In addition, an attempt will be made to make non-{@code public} - * methods accessible, thus allowing one to invoke {@code protected}, - * {@code private}, and package-private setter methods. - *

In addition, this method supports JavaBean-style property - * names. For example, if you wish to set the {@code name} property on the - * target object, you may pass either "name" or - * "setName" as the method name. + *

This method delegates to + * {@link #invokeSetterMethod(Object, String, Object, Class)}, supplying + * {@code null} for the parameter type. * @param target the target object on which to invoke the specified setter * method * @param name the name of the setter method to invoke or the corresponding * property name * @param value the value to provide to the setter method - * @see ReflectionUtils#findMethod(Class, String, Class[]) - * @see ReflectionUtils#makeAccessible(Method) - * @see ReflectionUtils#invokeMethod(Method, Object, Object[]) */ public static void invokeSetterMethod(Object target, String name, Object value) { invokeSetterMethod(target, name, value, null); @@ -317,19 +310,24 @@ public abstract class ReflectionTestUtils { * method. In addition, an attempt will be made to make non-{@code public} * methods accessible, thus allowing one to invoke {@code protected}, * {@code private}, and package-private setter methods. - *

In addition, this method supports JavaBean-style property - * names. For example, if you wish to set the {@code name} property on the - * target object, you may pass either "name" or - * "setName" as the method name. + *

This method also supports JavaBean-style property names. For + * example, if you wish to set the {@code name} property on the target object, + * you may pass either {@code "name"} or {@code "setName"} as the method name. + *

As of Spring Framework 6.2, if the supplied target object is a CGLIB + * proxy which does not intercept the setter method, the proxy will be + * {@linkplain AopTestUtils#getUltimateTargetObject unwrapped} allowing the + * setter method to be invoked directly on the ultimate target of the proxy. * @param target the target object on which to invoke the specified setter * method * @param name the name of the setter method to invoke or the corresponding * property name * @param value the value to provide to the setter method * @param type the formal parameter type declared by the setter method + * (may be {@code null} to indicate any type) * @see ReflectionUtils#findMethod(Class, String, Class[]) * @see ReflectionUtils#makeAccessible(Method) * @see ReflectionUtils#invokeMethod(Method, Object, Object[]) + * @see AopTestUtils#getUltimateTargetObject(Object) */ public static void invokeSetterMethod(Object target, String name, @Nullable Object value, @Nullable Class type) { Assert.notNull(target, "Target object must not be null"); @@ -357,6 +355,14 @@ public abstract class ReflectionTestUtils { safeToString(target), value)); } + if (springAopPresent) { + // If the target is a CGLIB proxy which does not intercept the method, invoke the + // method on the ultimate target. + if (isCglibProxyThatDoesNotInterceptMethod(target, method)) { + target = AopTestUtils.getUltimateTargetObject(target); + } + } + ReflectionUtils.makeAccessible(method); ReflectionUtils.invokeMethod(method, target, value); } @@ -368,10 +374,13 @@ public abstract class ReflectionTestUtils { * method. In addition, an attempt will be made to make non-{@code public} * methods accessible, thus allowing one to invoke {@code protected}, * {@code private}, and package-private getter methods. - *

In addition, this method supports JavaBean-style property - * names. For example, if you wish to get the {@code name} property on the - * target object, you may pass either "name" or - * "getName" as the method name. + *

This method also supports JavaBean-style property names. For + * example, if you wish to get the {@code name} property on the target object, + * you may pass either {@code "name"} or {@code "getName"} as the method name. + *

As of Spring Framework 6.2, if the supplied target object is a CGLIB + * proxy which does not intercept the getter method, the proxy will be + * {@linkplain AopTestUtils#getUltimateTargetObject unwrapped} allowing the + * getter method to be invoked directly on the ultimate target of the proxy. * @param target the target object on which to invoke the specified getter * method * @param name the name of the getter method to invoke or the corresponding @@ -380,6 +389,7 @@ public abstract class ReflectionTestUtils { * @see ReflectionUtils#findMethod(Class, String, Class[]) * @see ReflectionUtils#makeAccessible(Method) * @see ReflectionUtils#invokeMethod(Method, Object, Object[]) + * @see AopTestUtils#getUltimateTargetObject(Object) */ @Nullable public static Object invokeGetterMethod(Object target, String name) { @@ -400,6 +410,14 @@ public abstract class ReflectionTestUtils { "Could not find getter method '%s' on %s", getterMethodName, safeToString(target))); } + if (springAopPresent) { + // If the target is a CGLIB proxy which does not intercept the method, invoke the + // method on the ultimate target. + if (isCglibProxyThatDoesNotInterceptMethod(target, method)) { + target = AopTestUtils.getUltimateTargetObject(target); + } + } + if (logger.isDebugEnabled()) { logger.debug(String.format("Invoking getter method '%s' on %s", getterMethodName, safeToString(target))); } @@ -418,10 +436,6 @@ public abstract class ReflectionTestUtils { * @return the invocation result, if any * @see #invokeMethod(Class, String, Object...) * @see #invokeMethod(Object, Class, String, Object...) - * @see MethodInvoker - * @see ReflectionUtils#makeAccessible(Method) - * @see ReflectionUtils#invokeMethod(Method, Object, Object[]) - * @see ReflectionUtils#handleReflectionException(Exception) */ @Nullable public static T invokeMethod(Object target, String name, Object... args) { @@ -441,10 +455,6 @@ public abstract class ReflectionTestUtils { * @since 5.2 * @see #invokeMethod(Object, String, Object...) * @see #invokeMethod(Object, Class, String, Object...) - * @see MethodInvoker - * @see ReflectionUtils#makeAccessible(Method) - * @see ReflectionUtils#invokeMethod(Method, Object, Object[]) - * @see ReflectionUtils#handleReflectionException(Exception) */ @Nullable public static T invokeMethod(Class targetClass, String name, Object... args) { @@ -459,6 +469,10 @@ public abstract class ReflectionTestUtils { * method. In addition, an attempt will be made to make non-{@code public} * methods accessible, thus allowing one to invoke {@code protected}, * {@code private}, and package-private methods. + *

As of Spring Framework 6.2, if the supplied target object is a CGLIB + * proxy which does not intercept the method, the proxy will be + * {@linkplain AopTestUtils#getUltimateTargetObject unwrapped} allowing the + * method to be invoked directly on the ultimate target of the proxy. * @param targetObject the target object on which to invoke the method; may * be {@code null} if the method is static * @param targetClass the target class on which to invoke the method; may @@ -471,8 +485,8 @@ public abstract class ReflectionTestUtils { * @see #invokeMethod(Class, String, Object...) * @see MethodInvoker * @see ReflectionUtils#makeAccessible(Method) - * @see ReflectionUtils#invokeMethod(Method, Object, Object[]) * @see ReflectionUtils#handleReflectionException(Exception) + * @see AopTestUtils#getUltimateTargetObject(Object) */ @SuppressWarnings("unchecked") @Nullable @@ -493,6 +507,15 @@ public abstract class ReflectionTestUtils { methodInvoker.setArguments(args); methodInvoker.prepare(); + if (targetObject != null && springAopPresent) { + // If the target is a CGLIB proxy which does not intercept the method, invoke the + // method on the ultimate target. + if (isCglibProxyThatDoesNotInterceptMethod(targetObject, methodInvoker.getPreparedMethod())) { + targetObject = AopTestUtils.getUltimateTargetObject(targetObject); + methodInvoker.setTargetObject(targetObject); + } + } + if (logger.isDebugEnabled()) { logger.debug(String.format("Invoking method '%s' on %s or %s with arguments %s", name, safeToString(targetObject), safeToString(targetClass), ObjectUtils.nullSafeToString(args))); @@ -520,4 +543,13 @@ public abstract class ReflectionTestUtils { return String.format("target class [%s]", (clazz != null ? clazz.getName() : null)); } + /** + * Determine if the supplied target object is a CBLIB proxy that does not intercept the + * supplied method. + * @since 6.2 + */ + private static boolean isCglibProxyThatDoesNotInterceptMethod(Object target, Method method) { + return (AopUtils.isCglibProxy(target) && !method.getDeclaringClass().equals(target.getClass())); + } + } diff --git a/spring-test/src/test/java/org/springframework/test/util/ReflectionTestUtilsTests.java b/spring-test/src/test/java/org/springframework/test/util/ReflectionTestUtilsTests.java index b8b95cc93b..5ba8bb36ab 100644 --- a/spring-test/src/test/java/org/springframework/test/util/ReflectionTestUtilsTests.java +++ b/spring-test/src/test/java/org/springframework/test/util/ReflectionTestUtilsTests.java @@ -32,6 +32,7 @@ import org.springframework.test.util.subpackage.StaticMethods; 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.SoftAssertions.assertSoftly; import static org.springframework.test.util.ReflectionTestUtils.getField; import static org.springframework.test.util.ReflectionTestUtils.invokeGetterMethod; import static org.springframework.test.util.ReflectionTestUtils.invokeMethod; @@ -48,7 +49,7 @@ class ReflectionTestUtilsTests { private static final Float PI = (float) 22 / 7; - private final Person person = new PersonEntity(); + private final PersonEntity person = new PersonEntity(); private final Component component = new Component(); @@ -156,7 +157,7 @@ class ReflectionTestUtilsTests { assertThat(person.getAge()).as("age (private field)").isEqualTo(42); assertThat(person.getEyeColor()).as("eye color (package private field)").isEqualTo("blue"); assertThat(person.likesPets()).as("'likes pets' flag (package private boolean field)").isTrue(); - assertThat(person.getFavoriteNumber()).as("'favorite number' (package field)").isEqualTo(PI); + assertThat(person.getFavoriteNumber()).as("'favorite number' (private field)").isEqualTo(PI); } private static void assertSetFieldAndGetFieldBehaviorForProxy(Person proxy, Person target) { @@ -168,7 +169,7 @@ class ReflectionTestUtilsTests { assertThat(target.getAge()).as("age (private field)").isEqualTo(42); assertThat(target.getEyeColor()).as("eye color (package private field)").isEqualTo("blue"); assertThat(target.likesPets()).as("'likes pets' flag (package private boolean field)").isTrue(); - assertThat(target.getFavoriteNumber()).as("'favorite number' (package field)").isEqualTo(PI); + assertThat(target.getFavoriteNumber()).as("'favorite number' (private field)").isEqualTo(PI); } @Test @@ -188,7 +189,7 @@ class ReflectionTestUtilsTests { assertThat(person.getName()).as("name (protected field)").isNull(); assertThat(person.getEyeColor()).as("eye color (package private field)").isNull(); - assertThat(person.getFavoriteNumber()).as("'favorite number' (package field)").isNull(); + assertThat(person.getFavoriteNumber()).as("'favorite number' (private field)").isNull(); } @Test @@ -295,6 +296,22 @@ class ReflectionTestUtilsTests { assertThat(invokeGetterMethod(person, "favoriteNumber")).isEqualTo(PI); } + @Test // gh-33429 + void invokingPrivateGetterMethodViaCglibProxyInvokesMethodOnUltimateTarget() { + this.person.setPrivateEye("I"); + ProxyFactory pf = new ProxyFactory(this.person); + pf.setProxyTargetClass(true); + PersonEntity proxy = (PersonEntity) pf.getProxy(); + assertThat(AopUtils.isCglibProxy(proxy)).as("Proxy is a CGLIB proxy").isTrue(); + + assertSoftly(softly -> { + softly.assertThat(getField(this.person, "privateEye")).as("'privateEye' (private field in target)").isEqualTo("I"); + softly.assertThat(getField(proxy, "privateEye")).as("'privateEye' (private field in proxy)").isEqualTo("I"); + softly.assertThat(invokeGetterMethod(this.person, "privateEye")).as("'privateEye' (getter on target)").isEqualTo("I"); + softly.assertThat(invokeGetterMethod(proxy, "privateEye")).as("'privateEye' (getter on proxy)").isEqualTo("I"); + }); + } + @Test void invokeSetterMethodWithNullValuesForNonPrimitives() { invokeSetterMethod(person, "name", null, String.class); @@ -306,6 +323,41 @@ class ReflectionTestUtilsTests { assertThat(person.getFavoriteNumber()).as("'favorite number' (protected method for a Number)").isNull(); } + @Test // gh-33429 + void invokingPrivateSetterMethodViaCglibProxyInvokesMethodOnUltimateTarget() { + ProxyFactory pf = new ProxyFactory(this.person); + pf.setProxyTargetClass(true); + PersonEntity proxy = (PersonEntity) pf.getProxy(); + assertThat(AopUtils.isCglibProxy(proxy)).as("Proxy is a CGLIB proxy").isTrue(); + + // Set reflectively + invokeSetterMethod(proxy, "favoriteNumber", PI, Number.class); + + assertSoftly(softly -> { + softly.assertThat(getField(proxy, "favoriteNumber")).as("'favorite number' (private field)").isEqualTo(PI); + softly.assertThat(proxy.getFavoriteNumber()).as("'favorite number' (getter on proxy)").isEqualTo(PI); + softly.assertThat(this.person.getFavoriteNumber()).as("'favorite number' (getter on target)").isEqualTo(PI); + }); + } + + @Test // gh-33429 + void invokingFinalSetterMethodViaCglibProxyInvokesMethodOnUltimateTarget() { + ProxyFactory pf = new ProxyFactory(this.person); + pf.setProxyTargetClass(true); + PersonEntity proxy = (PersonEntity) pf.getProxy(); + assertThat(AopUtils.isCglibProxy(proxy)).as("Proxy is a CGLIB proxy").isTrue(); + assertThat(proxy.getPuzzle()).as("puzzle").isNull(); + + // Set reflectively + invokeSetterMethod(proxy, "puzzle", "enigma", String.class); + + assertSoftly(softly -> { + softly.assertThat(getField(proxy, "puzzle")).as("'puzzle' (private field)").isEqualTo("enigma"); + softly.assertThat(proxy.getPuzzle()).as("'puzzle' (getter on proxy)").isEqualTo("enigma"); + softly.assertThat(this.person.getPuzzle()).as("'puzzle' (getter on target)").isEqualTo("enigma"); + }); + } + @Test void invokeSetterMethodWithNullValueForPrimitiveLong() { assertThatIllegalArgumentException().isThrownBy(() -> invokeSetterMethod(person, "id", null, long.class)); @@ -362,6 +414,23 @@ class ReflectionTestUtilsTests { assertThat(component.getText()).as("text").isNull(); } + @Test // gh-33429 + void invokingPrivateMethodViaCglibProxyInvokesMethodOnUltimateTarget() { + ProxyFactory pf = new ProxyFactory(this.person); + pf.setProxyTargetClass(true); + PersonEntity proxy = (PersonEntity) pf.getProxy(); + assertThat(AopUtils.isCglibProxy(proxy)).as("Proxy is a CGLIB proxy").isTrue(); + + // Set reflectively + invokeMethod(proxy, "setFavoriteNumber", PI); + + assertSoftly(softly -> { + softly.assertThat(getField(proxy, "favoriteNumber")).as("'favorite number' (private field)").isEqualTo(PI); + softly.assertThat(proxy.getFavoriteNumber()).as("'favorite number' (getter on proxy)").isEqualTo(PI); + softly.assertThat(this.person.getFavoriteNumber()).as("'favorite number' (getter on target)").isEqualTo(PI); + }); + } + @Test void invokeInitMethodBeforeAutowiring() { assertThatIllegalStateException() diff --git a/spring-test/src/test/java/org/springframework/test/util/subpackage/PersonEntity.java b/spring-test/src/test/java/org/springframework/test/util/subpackage/PersonEntity.java index 45b841983e..e247e92d85 100644 --- a/spring-test/src/test/java/org/springframework/test/util/subpackage/PersonEntity.java +++ b/spring-test/src/test/java/org/springframework/test/util/subpackage/PersonEntity.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 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. @@ -37,6 +37,10 @@ public class PersonEntity extends PersistentEntity implements Person { private Number favoriteNumber; + private String puzzle; + + private String privateEye; + @Override public String getName() { @@ -44,7 +48,7 @@ public class PersonEntity extends PersistentEntity implements Person { } @SuppressWarnings("unused") - private void setName(final String name) { + private void setName(String name) { this.name = name; } @@ -53,7 +57,7 @@ public class PersonEntity extends PersistentEntity implements Person { return this.age; } - protected void setAge(final int age) { + protected void setAge(int age) { this.age = age; } @@ -62,7 +66,7 @@ public class PersonEntity extends PersistentEntity implements Person { return this.eyeColor; } - void setEyeColor(final String eyeColor) { + void setEyeColor(String eyeColor) { this.eyeColor = eyeColor; } @@ -71,7 +75,7 @@ public class PersonEntity extends PersistentEntity implements Person { return this.likesPets; } - protected void setLikesPets(final boolean likesPets) { + protected void setLikesPets(boolean likesPets) { this.likesPets = likesPets; } @@ -80,10 +84,28 @@ public class PersonEntity extends PersistentEntity implements Person { return this.favoriteNumber; } - protected void setFavoriteNumber(Number favoriteNumber) { + @SuppressWarnings("unused") + private void setFavoriteNumber(Number favoriteNumber) { this.favoriteNumber = favoriteNumber; } + public String getPuzzle() { + return this.puzzle; + } + + public final void setPuzzle(String puzzle) { + this.puzzle = puzzle; + } + + @SuppressWarnings("unused") + private String getPrivateEye() { + return this.privateEye; + } + + public void setPrivateEye(String privateEye) { + this.privateEye = privateEye; + } + @Override public String toString() { // @formatter:off @@ -94,6 +116,8 @@ public class PersonEntity extends PersistentEntity implements Person { .append("eyeColor", this.eyeColor) .append("likesPets", this.likesPets) .append("favoriteNumber", this.favoriteNumber) + .append("puzzle", this.puzzle) + .append("privateEye", this.privateEye) .toString(); // @formatter:on }