Invoke init/destroy/SpEL methods via public types whenever possible

Prior to this commit, when invoking init methods and destroy methods
for beans as well as methods within Spring Expression Language (SpEL)
expressions via reflection, we invoked them based on the "interface
method" returned from ClassUtils.getInterfaceMethodIfPossible(). That
works well for finding methods defined in an interface, but it does not
find public methods defined in a public superclass.

For example, in a SpEL expression it was previously impossible to
invoke toString() on a non-public type from a different module. This
could be seen when attempting to invoke toString() on an unmodifiable
list created by Collections.unmodifiableList(...). Doing so resulted in
an InaccessibleObjectException.

Although users can address that by adding an appropriate --add-opens
declaration, such as --add-opens java.base/java.util=ALL-UNNAMED, it is
better if applications do not have to add an --add-opens declaration
for such use cases in SpEL. The same applies to init methods and
destroy methods for beans.

This commit therefore introduces a new
getPubliclyAccessibleMethodIfPossible() method in ClassUtils which
serves as a replacement for getInterfaceMethodIfPossible().

This new method finds the first publicly accessible method in the
supplied method's type hierarchy that has a method signature equivalent
to the supplied method. If the supplied method is public and declared
in a public type, the supplied method will be returned. Otherwise, this
method recursively searches the class hierarchy and implemented
interfaces for an equivalent method that is public and declared in a
public type. If a publicly accessible equivalent method cannot be
found, the supplied method will be returned, indicating that no such
equivalent method exists.

All usage of getInterfaceMethodIfPossible() has been replaced with
getPubliclyAccessibleMethodIfPossible() in spring-beans and
spring-expression. In addition, getInterfaceMethodIfPossible() has been
marked as deprecated in favor of the new method.

As a bonus, the introduction of getPubliclyAccessibleMethodIfPossible()
allows us to delete a fair amount of obsolete code within the SpEL
infrastructure.

See gh-29857
Closes gh-33216
This commit is contained in:
Sam Brannen 2024-07-14 12:01:25 +02:00
parent cac623b3f4
commit 47f88e123f
15 changed files with 519 additions and 171 deletions

View File

@ -176,9 +176,9 @@ class BeanDefinitionPropertiesCodeGenerator {
Method method = ReflectionUtils.findMethod(methodDeclaringClass, methodName);
if (method != null) {
this.hints.reflection().registerMethod(method, ExecutableMode.INVOKE);
Method interfaceMethod = ClassUtils.getInterfaceMethodIfPossible(method, beanUserClass);
if (!interfaceMethod.equals(method)) {
this.hints.reflection().registerMethod(interfaceMethod, ExecutableMode.INVOKE);
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(method, beanUserClass);
if (!publiclyAccessibleMethod.equals(method)) {
this.hints.reflection().registerMethod(publiclyAccessibleMethod, ExecutableMode.INVOKE);
}
}
}

View File

@ -1899,7 +1899,7 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
if (logger.isTraceEnabled()) {
logger.trace("Invoking init method '" + methodName + "' on bean with name '" + beanName + "'");
}
Method methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(initMethod, beanClass);
Method methodToInvoke = ClassUtils.getPubliclyAccessibleMethodIfPossible(initMethod, beanClass);
try {
ReflectionUtils.makeAccessible(methodToInvoke);

View File

@ -147,7 +147,7 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
beanName + "' has a non-boolean parameter - not supported as destroy method");
}
}
destroyMethod = ClassUtils.getInterfaceMethodIfPossible(destroyMethod, bean.getClass());
destroyMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(destroyMethod, bean.getClass());
destroyMethods.add(destroyMethod);
}
}
@ -253,8 +253,8 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
for (String destroyMethodName : this.destroyMethodNames) {
Method destroyMethod = determineDestroyMethod(destroyMethodName);
if (destroyMethod != null) {
invokeCustomDestroyMethod(
ClassUtils.getInterfaceMethodIfPossible(destroyMethod, this.bean.getClass()));
destroyMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(destroyMethod, this.bean.getClass());
invokeCustomDestroyMethod(destroyMethod);
}
}
}

View File

@ -631,7 +631,7 @@ class BeanDefinitionPropertiesCodeGeneratorTests {
}
interface Initializable {
public interface Initializable {
void initialize();
}
@ -643,7 +643,7 @@ class BeanDefinitionPropertiesCodeGeneratorTests {
}
}
interface Disposable {
public interface Disposable {
void dispose();
}

View File

@ -135,10 +135,17 @@ public abstract class ClassUtils {
private static final Set<Class<?>> javaLanguageInterfaces;
/**
* Cache for equivalent methods on an interface implemented by the declaring class.
* Cache for equivalent methods on a public interface implemented by the declaring class.
*/
private static final Map<Method, Method> interfaceMethodCache = new ConcurrentReferenceHashMap<>(256);
/**
* Cache for equivalent public methods in a public declaring type within the type hierarchy
* of the method's declaring class.
* @since 6.2
*/
private static final Map<Method, Method> publiclyAccessibleMethodCache = new ConcurrentReferenceHashMap<>(256);
static {
primitiveWrapperTypeMap.put(Boolean.class, boolean.class);
@ -1394,7 +1401,7 @@ public abstract class ClassUtils {
* @param method the method to be invoked, potentially from an implementation class
* @return the corresponding interface method, or the original method if none found
* @since 5.1
* @deprecated in favor of {@link #getInterfaceMethodIfPossible(Method, Class)}
* @deprecated in favor of {@link #getPubliclyAccessibleMethodIfPossible(Method, Class)}
*/
@Deprecated
public static Method getInterfaceMethodIfPossible(Method method) {
@ -1407,37 +1414,45 @@ public abstract class ClassUtils {
* Module System which allows the method to be invoked via reflection without an illegal
* access warning.
* @param method the method to be invoked, potentially from an implementation class
* @param targetClass the target class to check for declared interfaces
* @param targetClass the target class to invoke the method on, or {@code null} if unknown
* @return the corresponding interface method, or the original method if none found
* @since 5.3.16
* @see #getPubliclyAccessibleMethodIfPossible(Method, Class)
* @see #getMostSpecificMethod
* @deprecated in favor of {@link #getPubliclyAccessibleMethodIfPossible(Method, Class)}
*/
@Deprecated(since = "6.2")
public static Method getInterfaceMethodIfPossible(Method method, @Nullable Class<?> targetClass) {
if (!Modifier.isPublic(method.getModifiers()) || method.getDeclaringClass().isInterface()) {
Class<?> declaringClass = method.getDeclaringClass();
if (!Modifier.isPublic(method.getModifiers()) || declaringClass.isInterface()) {
return method;
}
String methodName = method.getName();
Class<?>[] parameterTypes = method.getParameterTypes();
// Try cached version of method in its declaring class
Method result = interfaceMethodCache.computeIfAbsent(method,
key -> findInterfaceMethodIfPossible(key, key.getParameterTypes(), key.getDeclaringClass(),
Object.class));
if (result == method && targetClass != null) {
key -> findInterfaceMethodIfPossible(methodName, parameterTypes, declaringClass, Object.class));
if (result == null && targetClass != null) {
// No interface method found yet -> try given target class (possibly a subclass of the
// declaring class, late-binding a base class method to a subclass-declared interface:
// see e.g. HashMap.HashIterator.hasNext)
result = findInterfaceMethodIfPossible(method, method.getParameterTypes(), targetClass,
method.getDeclaringClass());
result = findInterfaceMethodIfPossible(methodName, parameterTypes, targetClass, declaringClass);
}
return result;
return (result != null ? result : method);
}
private static Method findInterfaceMethodIfPossible(Method method, Class<?>[] parameterTypes,
@Nullable
private static Method findInterfaceMethodIfPossible(String methodName, Class<?>[] parameterTypes,
Class<?> startClass, Class<?> endClass) {
Class<?> current = startClass;
while (current != null && current != endClass) {
for (Class<?> ifc : current.getInterfaces()) {
try {
return ifc.getMethod(method.getName(), parameterTypes);
if (Modifier.isPublic(ifc.getModifiers())) {
return ifc.getMethod(methodName, parameterTypes);
}
}
catch (NoSuchMethodException ex) {
// ignore
@ -1445,7 +1460,72 @@ public abstract class ClassUtils {
}
current = current.getSuperclass();
}
return method;
return null;
}
/**
* Get the first publicly accessible method in the supplied method's type hierarchy that
* has a method signature equivalent to the supplied method, if possible.
* <p>If the supplied method is {@code public} and declared in a {@code public} type,
* the supplied method will be returned.
* <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.
* <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
* &mdash; for example, for use with the Spring Expression Language (SpEL) compiler.
* For example, if a non-public class overrides {@code toString()}, this method will
* traverse up the type hierarchy to find the first public type that declares the method
* (if there is one). For {@code toString()}, it may traverse as far as {@link Object}.
* @param method the method to be invoked, potentially from an implementation class
* @param targetClass the target class to invoke the method on, or {@code null} if unknown
* @return the corresponding publicly accessible method, or the original method if none
* found
* @since 6.2
* @see #getInterfaceMethodIfPossible(Method, Class)
* @see #getMostSpecificMethod(Method, Class)
*/
public static Method getPubliclyAccessibleMethodIfPossible(Method method, @Nullable Class<?> targetClass) {
Class<?> declaringClass = method.getDeclaringClass();
// If the method is not public, we can abort the search immediately; or if the method's
// declaring class is public, the method is already publicly accessible.
if (!Modifier.isPublic(method.getModifiers()) || Modifier.isPublic(declaringClass.getModifiers())) {
return method;
}
Method interfaceMethod = getInterfaceMethodIfPossible(method, targetClass);
// If we found a method in a public interface, return the interface method.
if (!interfaceMethod.equals(method)) {
return interfaceMethod;
}
Method result = publiclyAccessibleMethodCache.computeIfAbsent(method,
key -> findPubliclyAccessibleMethodIfPossible(key.getName(), key.getParameterTypes(), declaringClass));
return (result != null ? result : method);
}
@Nullable
private static Method findPubliclyAccessibleMethodIfPossible(
String methodName, Class<?>[] parameterTypes, Class<?> declaringClass) {
Class<?> current = declaringClass.getSuperclass();
while (current != null) {
if (Modifier.isPublic(current.getModifiers())) {
try {
return current.getDeclaredMethod(methodName, parameterTypes);
}
catch (NoSuchMethodException ex) {
// ignore
}
}
current = current.getSuperclass();
}
return null;
}
/**

View File

@ -23,12 +23,17 @@ import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.Proxy;
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.function.Supplier;
@ -37,6 +42,7 @@ import a.ClassHavingNestedClass;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;
@ -503,6 +509,278 @@ class ClassUtilsTests {
}
@Nested // gh-33216
class GetPubliclyAccessibleMethodTests {
@Test
void nonPublicMethod(TestInfo testInfo) {
Method originalMethod = testInfo.getTestMethod().get();
// Prerequisites for this use case:
assertNotPublic(originalMethod);
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
assertThat(publiclyAccessibleMethod).isSameAs(originalMethod);
assertNotPubliclyAccessible(publiclyAccessibleMethod);
}
@Test
// This method is intentionally public.
public void publicMethodInNonPublicType(TestInfo testInfo) {
Method originalMethod = testInfo.getTestMethod().get();
// Prerequisites for this use case:
assertPublic(originalMethod);
assertNotPublic(originalMethod.getDeclaringClass());
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
assertThat(publiclyAccessibleMethod).isSameAs(originalMethod);
assertNotPubliclyAccessible(publiclyAccessibleMethod);
}
@Test
void publicMethodInPublicType() throws Exception {
Class<?> originalType = String.class;
Method originalMethod = originalType.getDeclaredMethod("toString");
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(originalType);
assertThat(publiclyAccessibleMethod).isSameAs(originalMethod);
assertPubliclyAccessible(publiclyAccessibleMethod);
}
@Test
void publicInterfaceMethodInPublicType() throws Exception {
Class<?> originalType = ArrayList.class;
Method originalMethod = originalType.getDeclaredMethod("size");
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
// Should not find the interface method in List.
assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(originalType);
assertThat(publiclyAccessibleMethod).isSameAs(originalMethod);
assertPubliclyAccessible(publiclyAccessibleMethod);
}
@Test
void publicMethodInJavaLangObjectDeclaredInNonPublicType() throws Exception {
List<String> unmodifiableList = Collections.unmodifiableList(Arrays.asList("foo", "bar"));
Class<?> targetClass = unmodifiableList.getClass();
// Prerequisites for this use case:
assertNotPublic(targetClass);
Method originalMethod = targetClass.getMethod("toString");
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(Object.class);
assertThat(publiclyAccessibleMethod.getName()).isEqualTo("toString");
assertThat(publiclyAccessibleMethod.getParameterTypes()).isEmpty();
assertPubliclyAccessible(publiclyAccessibleMethod);
}
@Test
void publicMethodInJavaTimeZoneIdDeclaredInNonPublicSubclass() throws Exception {
// Returns a package-private java.time.ZoneRegion.
ZoneId zoneId = ZoneId.of("CET");
Class<?> targetClass = zoneId.getClass();
// Prerequisites for this use case:
assertNotPublic(targetClass);
Method originalMethod = targetClass.getDeclaredMethod("getId");
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(ZoneId.class);
assertThat(publiclyAccessibleMethod.getName()).isEqualTo("getId");
assertThat(publiclyAccessibleMethod.getParameterTypes()).isEmpty();
assertPubliclyAccessible(publiclyAccessibleMethod);
}
@Test
void publicInterfaceMethodDeclaredInNonPublicTypeWithLateBindingOfClassMethodToSubclassDeclaredInterface() throws Exception {
HashMap<String, String> hashMap = new HashMap<>();
// Returns a package-private java.util.HashMap.KeyIterator which extends java.util.HashMap.HashIterator
// which declares hasNext(), even though HashIterator does not implement Iterator. Rather, KeyIterator
// implements HashIterator.
Iterator<String> iterator = hashMap.keySet().iterator();
Class<?> targetClass = iterator.getClass();
// Prerequisites for this use case:
assertNotPublic(targetClass);
Method originalMethod = targetClass.getMethod("hasNext");
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, targetClass);
assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(Iterator.class);
assertThat(publiclyAccessibleMethod.getName()).isEqualTo("hasNext");
assertThat(publiclyAccessibleMethod.getParameterTypes()).isEmpty();
assertPubliclyAccessible(publiclyAccessibleMethod);
}
@Test
void privateSubclassOverridesPropertyInPublicInterface() throws Exception {
Method originalMethod = PrivateSubclass.class.getDeclaredMethod("getText");
// Prerequisite: type must not be public for this use case.
assertNotPublic(originalMethod.getDeclaringClass());
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(PublicInterface.class);
assertThat(publiclyAccessibleMethod.getName()).isEqualTo("getText");
assertThat(publiclyAccessibleMethod.getParameterTypes()).isEmpty();
assertPubliclyAccessible(publiclyAccessibleMethod);
}
@Test
void privateSubclassOverridesPropertyInPrivateInterface() throws Exception {
Method originalMethod = PrivateSubclass.class.getDeclaredMethod("getMessage");
// Prerequisite: type must not be public for this use case.
assertNotPublic(originalMethod.getDeclaringClass());
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
// Should not find the interface method in PrivateInterface.
assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(PublicSuperclass.class);
assertThat(publiclyAccessibleMethod.getName()).isEqualTo("getMessage");
assertThat(publiclyAccessibleMethod.getParameterTypes()).isEmpty();
assertPubliclyAccessible(publiclyAccessibleMethod);
}
@Test
void privateSubclassOverridesPropertyInPublicSuperclass() throws Exception {
Method originalMethod = PrivateSubclass.class.getDeclaredMethod("getNumber");
// Prerequisite: type must not be public for this use case.
assertNotPublic(originalMethod.getDeclaringClass());
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(PublicSuperclass.class);
assertThat(publiclyAccessibleMethod.getName()).isEqualTo("getNumber");
assertThat(publiclyAccessibleMethod.getParameterTypes()).isEmpty();
assertPubliclyAccessible(publiclyAccessibleMethod);
}
@Test
void packagePrivateSubclassOverridesMethodInPublicInterface() throws Exception {
List<String> unmodifiableList = Collections.unmodifiableList(Arrays.asList("foo", "bar"));
Class<?> targetClass = unmodifiableList.getClass();
// Prerequisites for this use case:
assertNotPublic(targetClass);
Method originalMethod = targetClass.getMethod("contains", Object.class);
// Prerequisite: type must not be public for this use case.
assertNotPublic(originalMethod.getDeclaringClass());
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(Collection.class);
assertThat(publiclyAccessibleMethod.getName()).isEqualTo("contains");
assertThat(publiclyAccessibleMethod.getParameterTypes()).containsExactly(Object.class);
assertPubliclyAccessible(publiclyAccessibleMethod);
}
@Test
void privateSubclassOverridesMethodInPrivateInterface() throws Exception {
Method originalMethod = PrivateSubclass.class.getMethod("greet", String.class);
// Prerequisite: type must not be public for this use case.
assertNotPublic(originalMethod.getDeclaringClass());
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(PublicSuperclass.class);
assertThat(publiclyAccessibleMethod.getName()).isEqualTo("greet");
assertThat(publiclyAccessibleMethod.getParameterTypes()).containsExactly(String.class);
assertPubliclyAccessible(publiclyAccessibleMethod);
}
@Test
void privateSubclassOverridesMethodInPublicSuperclass() throws Exception {
Method originalMethod = PrivateSubclass.class.getMethod("process", int.class);
// Prerequisite: type must not be public for this use case.
assertNotPublic(originalMethod.getDeclaringClass());
Method publiclyAccessibleMethod = ClassUtils.getPubliclyAccessibleMethodIfPossible(originalMethod, null);
assertThat(publiclyAccessibleMethod.getDeclaringClass()).isEqualTo(PublicSuperclass.class);
assertThat(publiclyAccessibleMethod.getName()).isEqualTo("process");
assertThat(publiclyAccessibleMethod.getParameterTypes()).containsExactly(int.class);
assertPubliclyAccessible(publiclyAccessibleMethod);
}
private static void assertPubliclyAccessible(Method method) {
assertPublic(method);
assertPublic(method.getDeclaringClass());
}
private static void assertNotPubliclyAccessible(Method method) {
assertThat(!isPublic(method) || !isPublic(method.getDeclaringClass()))
.as("%s must not be publicly accessible", method)
.isTrue();
}
private static void assertPublic(Member member) {
assertThat(isPublic(member)).as("%s must be public", member).isTrue();
}
private static void assertPublic(Class<?> clazz) {
assertThat(isPublic(clazz)).as("%s must be public", clazz).isTrue();
}
private static void assertNotPublic(Member member) {
assertThat(!isPublic(member)).as("%s must be not be public", member).isTrue();
}
private static void assertNotPublic(Class<?> clazz) {
assertThat(!isPublic(clazz)).as("%s must be not be public", clazz).isTrue();
}
private static boolean isPublic(Class<?> clazz) {
return Modifier.isPublic(clazz.getModifiers());
}
private static boolean isPublic(Member member) {
return Modifier.isPublic(member.getModifiers());
}
private interface PrivateInterface {
String getMessage();
String greet(String name);
}
private static class PrivateSubclass extends PublicSuperclass implements PublicInterface, PrivateInterface {
@Override
public int getNumber() {
return 2;
}
@Override
public String getMessage() {
return "hello";
}
@Override
public String greet(String name) {
return "Hello, " + name;
}
@Override
public int process(int num) {
return num * 2;
}
@Override
public String getText() {
return "enigma";
}
}
}
@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@ValueSource(classes = { Boolean.class, Character.class, Byte.class, Short.class,

View File

@ -0,0 +1,26 @@
/*
* 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.util;
/**
* This is intentionally a top-level public interface.
*/
public interface PublicInterface {
String getText();
}

View File

@ -0,0 +1,40 @@
/*
* 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.util;
/**
* This is intentionally a top-level public class.
*/
public class PublicSuperclass {
public String getMessage() {
return "goodbye";
}
public int getNumber() {
return 1;
}
public String greet(String name) {
return "Super, " + name;
}
public int process(int num) {
return num + 1;
}
}

View File

@ -18,12 +18,10 @@ package org.springframework.expression.spel;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Deque;
import java.util.List;
import java.util.Map;
import org.springframework.asm.ClassWriter;
import org.springframework.asm.MethodVisitor;
@ -31,9 +29,7 @@ import org.springframework.asm.Opcodes;
import org.springframework.lang.Contract;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.CollectionUtils;
import org.springframework.util.ConcurrentReferenceHashMap;
/**
* Manages the class being generated by the compilation process.
@ -49,14 +45,6 @@ import org.springframework.util.ConcurrentReferenceHashMap;
*/
public class CodeFlow implements Opcodes {
/**
* Cache for equivalent methods in a public declaring class in the type
* hierarchy of the method's declaring class.
* @since 6.2
*/
private static final Map<Method, Class<?>> publicDeclaringClassCache = new ConcurrentReferenceHashMap<>(256);
/**
* Name of the class being generated. Typically used when generating code
* that accesses freshly generated fields on the generated type.
@ -479,66 +467,6 @@ public class CodeFlow implements Opcodes {
}
}
/**
* Find the first public class or interface in the method's class hierarchy
* that declares the supplied method.
* <p>Sometimes the reflective method discovery logic finds a suitable method
* that can easily be called via reflection but cannot be called from generated
* code when compiling the expression because of visibility restrictions. For
* example, if a non-public class overrides {@code toString()}, this method
* will traverse up the type hierarchy to find the first public type that
* declares the method (if there is one). For {@code toString()}, it may
* traverse as far as {@link Object}.
* @param method the method to process
* @return the public class or interface that declares the method, or
* {@code null} if no such public type could be found
* @since 6.2
*/
@Nullable
public static Class<?> findPublicDeclaringClass(Method method) {
return publicDeclaringClassCache.computeIfAbsent(method, key -> {
// If the method is already defined in a public type, return that type.
if (Modifier.isPublic(key.getDeclaringClass().getModifiers())) {
return key.getDeclaringClass();
}
Method interfaceMethod = ClassUtils.getInterfaceMethodIfPossible(key, null);
// If we found an interface method whose type is public, return the interface type.
if (!interfaceMethod.equals(key)) {
if (Modifier.isPublic(interfaceMethod.getDeclaringClass().getModifiers())) {
return interfaceMethod.getDeclaringClass();
}
}
// Attempt to search the type hierarchy.
Class<?> superclass = key.getDeclaringClass().getSuperclass();
if (superclass != null) {
return findPublicDeclaringClass(superclass, key.getName(), key.getParameterTypes());
}
// Otherwise, no public declaring class found.
return null;
});
}
@Nullable
private static Class<?> findPublicDeclaringClass(
Class<?> declaringClass, String methodName, Class<?>[] parameterTypes) {
if (Modifier.isPublic(declaringClass.getModifiers())) {
try {
declaringClass.getDeclaredMethod(methodName, parameterTypes);
return declaringClass;
}
catch (NoSuchMethodException ex) {
// Continue below...
}
}
Class<?> superclass = declaringClass.getSuperclass();
if (superclass != null) {
return findPublicDeclaringClass(superclass, methodName, parameterTypes);
}
return null;
}
/**
* Create the JVM signature descriptor for a method. This consists of the descriptors
* for the method parameters surrounded with parentheses, followed by the

View File

@ -301,9 +301,7 @@ public class MethodReference extends SpelNodeImpl {
}
Method method = executor.getMethod();
return ((Modifier.isPublic(method.getModifiers()) &&
(Modifier.isPublic(method.getDeclaringClass().getModifiers()) ||
executor.getPublicDeclaringClass() != null)));
return (Modifier.isPublic(method.getModifiers()) && executor.getPublicDeclaringClass() != null);
}
@Override
@ -314,13 +312,7 @@ public class MethodReference extends SpelNodeImpl {
}
Method method = methodExecutor.getMethod();
Class<?> publicDeclaringClass;
if (Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
publicDeclaringClass = method.getDeclaringClass();
}
else {
publicDeclaringClass = methodExecutor.getPublicDeclaringClass();
}
Class<?> publicDeclaringClass = methodExecutor.getPublicDeclaringClass();
Assert.state(publicDeclaringClass != null,
() -> "Failed to find public declaring class for method: " + method);

View File

@ -159,7 +159,7 @@ public class ReflectiveIndexAccessor implements CompilableIndexAccessor {
.formatted(readMethodName, getName(indexType), getName(targetType)));
}
this.readMethodToInvoke = ClassUtils.getInterfaceMethodIfPossible(this.readMethod, targetType);
this.readMethodToInvoke = ClassUtils.getPubliclyAccessibleMethodIfPossible(this.readMethod, targetType);
ReflectionUtils.makeAccessible(this.readMethodToInvoke);
if (writeMethodName != null) {
@ -173,7 +173,7 @@ public class ReflectiveIndexAccessor implements CompilableIndexAccessor {
.formatted(writeMethodName, getName(indexType), getName(indexedValueType),
getName(targetType)));
}
this.writeMethodToInvoke = ClassUtils.getInterfaceMethodIfPossible(writeMethod, targetType);
this.writeMethodToInvoke = ClassUtils.getPubliclyAccessibleMethodIfPossible(writeMethod, targetType);
ReflectionUtils.makeAccessible(this.writeMethodToInvoke);
}
else {
@ -250,10 +250,7 @@ public class ReflectiveIndexAccessor implements CompilableIndexAccessor {
public void generateCode(SpelNode index, MethodVisitor mv, CodeFlow cf) {
// Find the public declaring class.
Class<?> publicDeclaringClass = this.readMethodToInvoke.getDeclaringClass();
if (!Modifier.isPublic(publicDeclaringClass.getModifiers())) {
publicDeclaringClass = CodeFlow.findPublicDeclaringClass(this.readMethod);
}
Assert.state(publicDeclaringClass != null && Modifier.isPublic(publicDeclaringClass.getModifiers()),
Assert.state(Modifier.isPublic(publicDeclaringClass.getModifiers()),
() -> "Failed to find public declaring class for read-method: " + this.readMethod);
String classDesc = publicDeclaringClass.getName().replace('.', '/');

View File

@ -17,6 +17,7 @@
package org.springframework.expression.spel.support;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import org.springframework.core.MethodParameter;
import org.springframework.core.convert.TypeDescriptor;
@ -24,7 +25,6 @@ import org.springframework.expression.AccessException;
import org.springframework.expression.EvaluationContext;
import org.springframework.expression.MethodExecutor;
import org.springframework.expression.TypedValue;
import org.springframework.expression.spel.CodeFlow;
import org.springframework.lang.Nullable;
import org.springframework.util.ClassUtils;
import org.springframework.util.ReflectionUtils;
@ -42,18 +42,15 @@ public class ReflectiveMethodExecutor implements MethodExecutor {
private final Method originalMethod;
/**
* The method to invoke via reflection, which is not necessarily the method
* to invoke in a compiled expression.
* The method to invoke via reflection or in a compiled expression.
*/
private final Method methodToInvoke;
@Nullable
private final Integer varargsPosition;
private boolean computedPublicDeclaringClass = false;
@Nullable
private Class<?> publicDeclaringClass;
private final Class<?> publicDeclaringClass;
private boolean argumentConversionOccurred = false;
@ -69,18 +66,15 @@ public class ReflectiveMethodExecutor implements MethodExecutor {
/**
* Create a new executor for the given method.
* @param method the method to invoke
* @param targetClass the target class to invoke the method on
* @param targetClass the target class to invoke the method on, or {@code null} if unknown
* @since 5.3.16
*/
public ReflectiveMethodExecutor(Method method, @Nullable Class<?> targetClass) {
this.originalMethod = method;
this.methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(method, targetClass);
if (method.isVarArgs()) {
this.varargsPosition = method.getParameterCount() - 1;
}
else {
this.varargsPosition = null;
}
this.methodToInvoke = ClassUtils.getPubliclyAccessibleMethodIfPossible(method, targetClass);
Class<?> declaringClass = this.methodToInvoke.getDeclaringClass();
this.publicDeclaringClass = (Modifier.isPublic(declaringClass.getModifiers()) ? declaringClass : null);
this.varargsPosition = (method.isVarArgs() ? method.getParameterCount() - 1 : null);
}
@ -92,19 +86,15 @@ public class ReflectiveMethodExecutor implements MethodExecutor {
}
/**
* Find a public class or interface in the method's class hierarchy that
* declares the {@linkplain #getMethod() original method}.
* <p>See {@link CodeFlow#findPublicDeclaringClass(Method)} for
* Get the public class or interface in the method's type hierarchy that declares the
* {@linkplain #getMethod() original method}.
* <p>See {@link ClassUtils#getPubliclyAccessibleMethodIfPossible(Method, Class)} for
* details.
* @return the public class or interface that declares the method, or
* {@code null} if no such public type could be found
* @return the public class or interface that declares the method, or {@code null} if
* no such public type could be found
*/
@Nullable
public Class<?> getPublicDeclaringClass() {
if (!this.computedPublicDeclaringClass) {
this.publicDeclaringClass = CodeFlow.findPublicDeclaringClass(this.originalMethod);
this.computedPublicDeclaringClass = true;
}
return this.publicDeclaringClass;
}

View File

@ -136,8 +136,8 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
// The readerCache will only contain gettable properties (let's not worry about setters for now).
Property property = new Property(type, method, null);
TypeDescriptor typeDescriptor = new TypeDescriptor(property);
Method methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(method, type);
this.readerCache.put(cacheKey, new InvokerPair(methodToInvoke, typeDescriptor, method));
Method methodToInvoke = ClassUtils.getPubliclyAccessibleMethodIfPossible(method, type);
this.readerCache.put(cacheKey, new InvokerPair(methodToInvoke, typeDescriptor));
this.typeDescriptorCache.put(cacheKey, typeDescriptor);
return true;
}
@ -180,8 +180,8 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
// The readerCache will only contain gettable properties (let's not worry about setters for now).
Property property = new Property(type, method, null);
TypeDescriptor typeDescriptor = new TypeDescriptor(property);
methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(method, type);
invoker = new InvokerPair(methodToInvoke, typeDescriptor, method);
methodToInvoke = ClassUtils.getPubliclyAccessibleMethodIfPossible(method, type);
invoker = new InvokerPair(methodToInvoke, typeDescriptor);
this.readerCache.put(cacheKey, invoker);
}
}
@ -238,7 +238,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
// Treat it like a property
Property property = new Property(type, null, method);
TypeDescriptor typeDescriptor = new TypeDescriptor(property);
method = ClassUtils.getInterfaceMethodIfPossible(method, type);
method = ClassUtils.getPubliclyAccessibleMethodIfPossible(method, type);
this.writerCache.put(cacheKey, method);
this.typeDescriptorCache.put(cacheKey, typeDescriptor);
return true;
@ -287,7 +287,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
if (method == null) {
method = findSetterForProperty(name, type, target);
if (method != null) {
method = ClassUtils.getInterfaceMethodIfPossible(method, type);
method = ClassUtils.getPubliclyAccessibleMethodIfPossible(method, type);
cachedMember = method;
this.writerCache.put(cacheKey, cachedMember);
}
@ -512,7 +512,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
* field) to use each time {@link #read(EvaluationContext, Object, String)}
* is called.
* <p>This method will return this {@code ReflectivePropertyAccessor} instance
* if it is unable to build a optimized accessor.
* if it is unable to build an optimized accessor.
* <p>Note: An optimized accessor is currently only usable for read attempts.
* Do not call this method if you need a read-write accessor.
*/
@ -536,9 +536,9 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
method = findGetterForProperty(name, type, target);
if (method != null) {
TypeDescriptor typeDescriptor = new TypeDescriptor(new MethodParameter(method, -1));
Method methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(method, type);
invokerPair = new InvokerPair(methodToInvoke, typeDescriptor, method);
Method methodToInvoke = ClassUtils.getPubliclyAccessibleMethodIfPossible(method, type);
ReflectionUtils.makeAccessible(methodToInvoke);
invokerPair = new InvokerPair(methodToInvoke, typeDescriptor);
this.readerCache.put(cacheKey, invokerPair);
}
}
@ -552,8 +552,8 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
if (field == null) {
field = findField(name, type, target instanceof Class);
if (field != null) {
invokerPair = new InvokerPair(field, new TypeDescriptor(field));
ReflectionUtils.makeAccessible(field);
invokerPair = new InvokerPair(field, new TypeDescriptor(field));
this.readerCache.put(cacheKey, invokerPair);
}
}
@ -576,13 +576,8 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
/**
* Captures the member (method/field) to call reflectively to access a property value
* and the type descriptor for the value returned by the reflective call.
* <p>The {@code originalMethod} is only used if the member is a method.
*/
private record InvokerPair(Member member, TypeDescriptor typeDescriptor, @Nullable Method originalMethod) {
InvokerPair(Member member, TypeDescriptor typeDescriptor) {
this(member, typeDescriptor, null);
}
private record InvokerPair(Member member, TypeDescriptor typeDescriptor) {
}
private record PropertyCacheKey(Class<?> clazz, String property, boolean targetIsClass)
@ -617,16 +612,10 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
private final TypeDescriptor typeDescriptor;
/**
* The original method, or {@code null} if the member is not a method.
*/
@Nullable
private final Method originalMethod;
OptimalPropertyAccessor(InvokerPair invokerPair) {
this.member = invokerPair.member;
this.typeDescriptor = invokerPair.typeDescriptor;
this.originalMethod = invokerPair.originalMethod;
}
@Override
@ -695,14 +684,8 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
@Override
public boolean isCompilable() {
if (Modifier.isPublic(this.member.getModifiers()) &&
Modifier.isPublic(this.member.getDeclaringClass().getModifiers())) {
return true;
}
if (this.originalMethod != null) {
return (CodeFlow.findPublicDeclaringClass(this.originalMethod) != null);
}
return false;
return (Modifier.isPublic(this.member.getModifiers()) &&
Modifier.isPublic(this.member.getDeclaringClass().getModifiers()));
}
@Override
@ -718,12 +701,8 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
@Override
public void generateCode(String propertyName, MethodVisitor mv, CodeFlow cf) {
Class<?> publicDeclaringClass = this.member.getDeclaringClass();
if (!Modifier.isPublic(publicDeclaringClass.getModifiers()) && this.originalMethod != null) {
publicDeclaringClass = CodeFlow.findPublicDeclaringClass(this.originalMethod);
}
Assert.state(publicDeclaringClass != null && Modifier.isPublic(publicDeclaringClass.getModifiers()),
() -> "Failed to find public declaring class for: " +
(this.originalMethod != null ? this.originalMethod : this.member));
Assert.state(Modifier.isPublic(publicDeclaringClass.getModifiers()),
() -> "Failed to find public declaring class for: " + this.member);
String classDesc = publicDeclaringClass.getName().replace('.', '/');
boolean isStatic = Modifier.isStatic(this.member.getModifiers());

View File

@ -21,6 +21,8 @@ import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import org.junit.jupiter.api.Test;
@ -402,12 +404,32 @@ class MethodInvocationTests extends AbstractExpressionTests {
}
@Test
void methodOfClass() {
void methodOfJavaLangClass() {
Expression expression = parser.parseExpression("getName()");
Object value = expression.getValue(new StandardEvaluationContext(String.class));
assertThat(value).isEqualTo("java.lang.String");
}
@Test
void methodOfJavaLangObject() {
Expression expression = parser.parseExpression("toString()");
Object value = expression.getValue(new StandardEvaluationContext(42));
assertThat(value).isEqualTo("42");
}
@Test // gh-33216
void methodOfJavaLangObjectDeclaredInNonPublicType() throws Exception {
Expression expression = parser.parseExpression("toString()");
List<String> unmodifiableList = Collections.unmodifiableList(Arrays.asList("foo", "bar"));
Method toStringMethod = unmodifiableList.getClass().getMethod("toString");
// Prerequisite for this use case:
assertThat(toStringMethod.getDeclaringClass()).isPackagePrivate();
Object value = expression.getValue(new StandardEvaluationContext(unmodifiableList));
assertThat(value).isEqualTo(unmodifiableList.toString());
}
@Test
void invokeMethodWithoutConversion() {
final BytesService service = new BytesService();

View File

@ -16,6 +16,7 @@
package org.springframework.expression.spel;
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@ -145,12 +146,27 @@ class PropertyAccessTests extends AbstractExpressionTests {
}
@Test
void accessingPropertyOfClass() {
void accessingPropertyOfJavaLangClass() {
Expression expression = parser.parseExpression("name");
Object value = expression.getValue(new StandardEvaluationContext(String.class));
assertThat(value).isEqualTo("java.lang.String");
}
@Test // gh-33216
void accessingPropertyOfJavaTimeZoneIdDeclaredInNonPublicSubclass() throws Exception {
String ID = "CET";
ZoneId zoneId = ZoneId.of(ID);
// Prerequisites for this use case:
assertThat(zoneId.getClass()).isPackagePrivate();
assertThat(zoneId.getId()).isEqualTo(ID);
Expression expression = parser.parseExpression("id");
String result = expression.getValue(new StandardEvaluationContext(zoneId), String.class);
assertThat(result).isEqualTo(ID);
}
@Test
void shouldAlwaysUsePropertyAccessorFromEvaluationContext() {
SpelExpressionParser parser = new SpelExpressionParser();