Abort search for static methods in getPubliclyAccessibleMethodIfPossible()

Prior to this commit, getPubliclyAccessibleMethodIfPossible() in
ClassUtils incorrectly returned a hidden static method as an
"equivalent" method for a static method with the same signature;
however, a static method cannot be overridden and therefore has no
"equivalent" method in a super type.

To fix that bug, this commit immediately aborts the search for an
"equivalent" publicly accessible method when the original method is a
static method.

See gh-33216
See gh-35189
See gh-35556
Closes gh-35667

Author: sbrannen

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

@@ -1483,15 +1483,16 @@ private static Method findInterfaceMethodIfPossible(String methodName, Class<?>[
 	}
 
 	/**
-	 * Get the closest publicly accessible (and exported) method in the supplied method's type
-	 * hierarchy that has a method signature equivalent to the supplied method, if possible.
-	 * <p>Otherwise, this method recursively searches the class hierarchy and implemented
-	 * interfaces for an equivalent method that is {@code public} and declared in a
-	 * {@code public} type.
-	 * <p>If a publicly accessible equivalent method cannot be found, the supplied method
-	 * will be returned, indicating that no such equivalent method exists. Consequently,
-	 * callers of this method must manually validate the accessibility of the returned method
-	 * if public access is a requirement.
+	 * Get the closest publicly accessible method in the supplied method's type hierarchy that
+	 * has a method signature equivalent to the supplied method, if possible.
+	 * <p>This method recursively searches the class hierarchy and implemented interfaces for
+	 * an equivalent method that is {@code public}, declared in a {@code public} type, and
+	 * {@linkplain Module#isExported(String, Module) exported} to {@code spring-core}.
+	 * <p>If the supplied method is not {@code public} or is {@code static}, or if a publicly
+	 * accessible equivalent method cannot be found, the supplied method will be returned,
+	 * indicating that no such equivalent method exists. Consequently, callers of this method
+	 * must manually validate the accessibility of the returned method if public access is a
+	 * requirement.
 	 * <p>This is particularly useful for arriving at a public exported type on the Java
 	 * Module System which allows the method to be invoked via reflection without an illegal
 	 * access warning. This is also useful for invoking methods via a public API in bytecode
@@ -1508,10 +1509,11 @@ private static Method findInterfaceMethodIfPossible(String methodName, Class<?>[
 	 */
 	public static Method getPubliclyAccessibleMethodIfPossible(Method method, @Nullable Class<?> targetClass) {
 		Class<?> declaringClass = method.getDeclaringClass();
-		// If the method is not public or its declaring class is public and exported already,
-		// we can abort the search immediately (avoiding reflection as well as cache access).
-		if (!Modifier.isPublic(method.getModifiers()) || (Modifier.isPublic(declaringClass.getModifiers()) &&
-				declaringClass.getModule().isExported(declaringClass.getPackageName(), ClassUtils.class.getModule()))) {
+		// If the method is not public, or it's static, or its declaring class is public and exported
+		// already, we can abort the search immediately (avoiding reflection as well as cache access).
+		if (!Modifier.isPublic(method.getModifiers()) || Modifier.isStatic(method.getModifiers()) ||
+				(Modifier.isPublic(declaringClass.getModifiers()) &&
+						declaringClass.getModule().isExported(declaringClass.getPackageName(), ClassUtils.class.getModule()))) {
 			return method;
 		}
 

spring-core/src/test/java/org/springframework/util/ClassUtilsTests.java

@@ -868,6 +868,49 @@ void privateSubclassOverridesMethodInPublicSuperclass() throws Exception {
 			assertPubliclyAccessible(publiclyAccessibleMethod);
 		}
 
+		@Test  // gh-35667
+		void staticMethodInPublicClass() throws Exception {
+			Method originalMethod = PublicSuperclass.class.getMethod("getCacheKey");
+
+			// Prerequisite: method must be public static for this use case.
+			assertPublic(originalMethod);
+			assertStatic(originalMethod);
+
+			Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
+			assertThat(publiclyAccessibleMethod).isSameAs(originalMethod);
+			assertPubliclyAccessible(publiclyAccessibleMethod);
+		}
+
+		@Test  // gh-35667
+		void publicSubclassHidesStaticMethodInPublicSuperclass() throws Exception {
+			Method originalMethod = PublicSubclass.class.getMethod("getCacheKey");
+
+			// Prerequisite: type must be public for this use case.
+			assertPublic(originalMethod.getDeclaringClass());
+			// Prerequisite: method must be public static for this use case.
+			assertPublic(originalMethod);
+			assertStatic(originalMethod);
+
+			Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
+			assertThat(publiclyAccessibleMethod).isSameAs(originalMethod);
+			assertPubliclyAccessible(publiclyAccessibleMethod);
+		}
+
+		@Test  // gh-35667
+		void privateSubclassHidesStaticMethodInPublicSuperclass() throws Exception {
+			Method originalMethod = PrivateSubclass.class.getMethod("getCacheKey");
+
+			// Prerequisite: type must not be public for this use case.
+			assertNotPublic(originalMethod.getDeclaringClass());
+			// Prerequisite: method must be public static for this use case.
+			assertPublic(originalMethod);
+			assertStatic(originalMethod);
+
+			Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
+			assertThat(publiclyAccessibleMethod).isSameAs(originalMethod);
+			assertNotPubliclyAccessible(publiclyAccessibleMethod);
+		}
+
 	}
 
 
@@ -914,6 +957,10 @@ private static boolean isPublic(Member member) {
 		return Modifier.isPublic(member.getModifiers());
 	}
 
+	private static void assertStatic(Member member) {
+		assertThat(Modifier.isStatic(member.getModifiers())).as("%s must be static", member).isTrue();
+	}
+
 
 	@Target(ElementType.METHOD)
 	@Retention(RetentionPolicy.RUNTIME)
@@ -1048,8 +1095,27 @@ private interface PrivateInterface {
 		String greet(String name);
 	}
 
+	public static class PublicSubclass extends PublicSuperclass {
+
+		/**
+		 * This method intentionally has the exact same signature as
+		 * {@link PublicSuperclass#getCacheKey()}.
+		 */
+		public static String getCacheKey() {
+			return "child";
+		}
+	}
+
 	private static class PrivateSubclass extends PublicSuperclass implements PublicInterface, PrivateInterface {
 
+		/**
+		 * This method intentionally has the exact same signature as
+		 * {@link PublicSuperclass#getCacheKey()}.
+		 */
+		public static String getCacheKey() {
+			return "child";
+		}
+
 		@Override
 		public int getNumber() {
 			return 2;

spring-core/src/test/java/org/springframework/util/PublicSuperclass.java

@@ -21,6 +21,15 @@
  */
 public class PublicSuperclass {
 
+	/**
+	 * This method intentionally has the exact same signature as
+	 * {@link org.springframework.util.ClassUtilsTests.PublicSubclass#getCacheKey()} and
+	 * {@link org.springframework.util.ClassUtilsTests.PrivateSubclass#getCacheKey()}.
+	 */
+	public static String getCacheKey() {
+		return "parent";
+	}
+
 	public String getMessage() {
 		return "goodbye";
 	}