From d01dca14e94b6ead5d8ed98c99ba4c7dfed0fefb Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 24 Jan 2022 17:03:16 +0100 Subject: [PATCH] Filter methods in Object in ReflectionUtils.USER_DECLARED_METHODS Prior to this commit, the USER_DECLARED_METHODS MethodFilter in ReflectionUtils did not actually filter methods declared in java.lang.Object as stated in its Javadoc. Consequently, if ReflectionUtils.doWithMethods was invoked with USER_DECLARED_METHODS and Object.class as the class to introspect, all non-bridge non-synthetic methods declared in java.lang.Object were passed to the supplied MethodCallback, which breaks the contract of USER_DECLARED_METHODS. In addition, if USER_DECLARED_METHODS was composed with a custom MethodFilter using `USER_DECLARED_METHODS.and()`, that composed method filter allowed all non-bridge non-synthetic methods declared in java.lang.Object to be passed to the supplied MethodCallback, which also breaks the contract of USER_DECLARED_METHODS. This behavior resulted in regressions in code that had previously used USER_DECLARED_METHODS by itself and then began using USER_DECLARED_METHODS in a composed filter. For example, since commit c419ea7ba7 ReflectiveAspectJAdvisorFactory has incorrectly processed methods in java.lang.Object as candidates for advice methods. This commit fixes this bug and associated regressions by ensuring that USER_DECLARED_METHODS actually filters methods declared in java.lang.Object. In addition, ReflectionUtils.doWithMethods now aborts its search algorithm early if invoked with the USER_DECLARED_METHODS filter and Object.class as the class to introspect. Closes gh-27970 --- .../springframework/util/ReflectionUtils.java | 6 ++++- .../util/ReflectionUtilsTests.java | 27 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java b/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java index 36dfaa1e5e..274cb71887 100644 --- a/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java @@ -51,7 +51,7 @@ public abstract class ReflectionUtils { * @since 3.0.5 */ public static final MethodFilter USER_DECLARED_METHODS = - (method -> !method.isBridge() && !method.isSynthetic()); + (method -> !method.isBridge() && !method.isSynthetic() && (method.getDeclaringClass() != Object.class)); /** * Pre-built FieldFilter that matches all non-static, non-final fields. @@ -354,6 +354,10 @@ public abstract class ReflectionUtils { * @throws IllegalStateException if introspection fails */ public static void doWithMethods(Class clazz, MethodCallback mc, @Nullable MethodFilter mf) { + if (mf == USER_DECLARED_METHODS && clazz == Object.class) { + // nothing to introspect + return; + } Method[] methods = getDeclaredMethods(clazz, false); for (Method method : methods) { if (mf != null && !mf.matches(method)) { diff --git a/spring-core/src/test/java/org/springframework/util/ReflectionUtilsTests.java b/spring-core/src/test/java/org/springframework/util/ReflectionUtilsTests.java index 06e2df6144..ef08975525 100644 --- a/spring-core/src/test/java/org/springframework/util/ReflectionUtilsTests.java +++ b/spring-core/src/test/java/org/springframework/util/ReflectionUtilsTests.java @@ -28,6 +28,7 @@ import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.tests.sample.objects.TestObject; +import org.springframework.util.ReflectionUtils.MethodFilter; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -193,6 +194,32 @@ class ReflectionUtilsTests { .as("Public, not protected").doesNotContain("hashCode", "absquatulate"); } + @Test + void doWithMethodsUsingUserDeclaredMethodsFilterStartingWithObject() { + ListSavingMethodCallback mc = new ListSavingMethodCallback(); + ReflectionUtils.doWithMethods(Object.class, mc, ReflectionUtils.USER_DECLARED_METHODS); + assertThat(mc.getMethodNames()).isEmpty(); + } + + @Test + void doWithMethodsUsingUserDeclaredMethodsFilterStartingWithTestObject() { + ListSavingMethodCallback mc = new ListSavingMethodCallback(); + ReflectionUtils.doWithMethods(TestObject.class, mc, ReflectionUtils.USER_DECLARED_METHODS); + assertThat(mc.getMethodNames()) + .as("user declared methods").contains("absquatulate", "compareTo", "getName", "setName", "getAge", "setAge", "getSpouse", "setSpouse") + .as("methods on Object").doesNotContain("equals", "hashCode", "toString", "clone", "finalize", "getClass", "notify", "notifyAll", "wait"); + } + + @Test + void doWithMethodsUsingUserDeclaredMethodsComposedFilter() { + ListSavingMethodCallback mc = new ListSavingMethodCallback(); + // "q" because both absquatulate() and equals() contain "q" + MethodFilter isSetterMethodOrNameContainsQ = m -> m.getName().startsWith("set") || m.getName().contains("q"); + MethodFilter methodFilter = ReflectionUtils.USER_DECLARED_METHODS.and(isSetterMethodOrNameContainsQ); + ReflectionUtils.doWithMethods(TestObject.class, mc, methodFilter); + assertThat(mc.getMethodNames()).containsExactlyInAnyOrder("setName", "setAge", "setSpouse", "absquatulate"); + } + @Test void doWithMethodsFindsDuplicatesInClassHierarchy() { ListSavingMethodCallback mc = new ListSavingMethodCallback();