Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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
Download Patch File Download Diff File Expand all files Collapse all files

72
spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java
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

12
spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java
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);

9
spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveIndexAccessor.java
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('.', '/');

36
spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java
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;
}

51
spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java
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());

24
spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java
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();

18
spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java
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();