root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 8 Packages Projects Releases Wiki Activity

Support SpEL compilation for public methods in private subtypes 

Although the Spring Expression Language (SpEL) generally does a good
job of locating the public declaring class or interface on which to
invoke a method in a compiled expression, prior to this commit there
were still a few unsupported use cases.

To address those remaining use cases, this commit ensures that methods
are invoked via a public interface or public superclass whenever
possible when compiling SpEL expressions.

See gh-29857
This commit is contained in:
Sam Brannen 2024-03-03 16:44:00 +01:00
parent 1ffffef85e
commit c79436f832
7 changed files with 357 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java
View File

@ -299,8 +299,10 @@ public class MethodReference extends SpelNodeImpl {
return false;
}
Class<?> clazz = executor.getMethod().getDeclaringClass();
return (Modifier.isPublic(clazz.getModifiers()) || executor.getPublicDeclaringClass() != null);
Method method = executor.getMethod();
return ((Modifier.isPublic(method.getModifiers()) &&
(Modifier.isPublic(method.getDeclaringClass().getModifiers()) ||
executor.getPublicDeclaringClass() != null)));
}
@Override
@ -310,6 +312,18 @@ public class MethodReference extends SpelNodeImpl {
throw new IllegalStateException("No applicable cached executor found: " + executorToCheck);
}
Method method = methodExecutor.getMethod();
Class<?> publicDeclaringClass;
if (Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
publicDeclaringClass = method.getDeclaringClass();
}
else {
publicDeclaringClass = methodExecutor.getPublicDeclaringClass();
}
Assert.state(publicDeclaringClass != null,
() -> "Failed to find public declaring class for method: " + method);
String classDesc = publicDeclaringClass.getName().replace('.', '/');
boolean isStatic = Modifier.isStatic(method.getModifiers());
String descriptor = cf.lastDescriptor();
@ -339,24 +353,15 @@ public class MethodReference extends SpelNodeImpl {
CodeFlow.insertBoxIfNecessary(mv, descriptor.charAt(0));
}
String classDesc;
if (Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
classDesc = method.getDeclaringClass().getName().replace('.', '/');
}
else {
Class<?> publicDeclaringClass = methodExecutor.getPublicDeclaringClass();
Assert.state(publicDeclaringClass != null, "No public declaring class");
classDesc = publicDeclaringClass.getName().replace('.', '/');
}
if (!isStatic && (descriptor == null || !descriptor.substring(1).equals(classDesc))) {
CodeFlow.insertCheckCast(mv, "L" + classDesc);
}
generateCodeForArguments(mv, cf, method, this.children);
int opcode = (isStatic ? INVOKESTATIC : method.isDefault() ? INVOKEINTERFACE : INVOKEVIRTUAL);
boolean isInterface = publicDeclaringClass.isInterface();
int opcode = (isStatic ? INVOKESTATIC : isInterface ? INVOKEINTERFACE : INVOKEVIRTUAL);
mv.visitMethodInsn(opcode, classDesc, method.getName(), CodeFlow.createSignatureDescriptor(method),
method.getDeclaringClass().isInterface());
isInterface);
cf.pushDescriptor(this.exitTypeDescriptor);
if (this.originalPrimitiveExitTypeDescriptor != null) {

71
spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java
View File

@ -21,7 +21,9 @@ import java.lang.invoke.MethodType;
import java.lang.reflect.Array;
import java.lang.reflect.Executable;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import org.springframework.core.MethodParameter;
@ -34,6 +36,7 @@ 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;
import org.springframework.util.MethodInvoker;
/**
@ -47,6 +50,14 @@ import org.springframework.util.MethodInvoker;
*/
public abstract class ReflectionHelper {
/**
* 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);
/**
* Compare argument arrays and return information about whether they match.
* <p>A supplied type converter and conversionAllowed flag allow for matches to take
@ -488,6 +499,66 @@ public abstract class ReflectionHelper {
return args;
}
/**
* 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;
}
/**
* Arguments match kinds.

37
spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java
View File

@ -17,7 +17,6 @@
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;
@ -34,6 +33,7 @@ import org.springframework.util.ReflectionUtils;
*
* @author Andy Clement
* @author Juergen Hoeller
* @author Sam Brannen
* @since 3.0
*/
public class ReflectiveMethodExecutor implements MethodExecutor {
@ -91,43 +91,22 @@ public class ReflectiveMethodExecutor implements MethodExecutor {
}
/**
* Find the first public class in the method's declaring class hierarchy that
* declares this 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 helper
* 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 Object.
* Find a public class or interface in the method's class hierarchy that
* declares the {@linkplain #getMethod() original method}.
* <p>See {@link ReflectionHelper#findPublicDeclaringClass(Method)} for
* details.
* @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 =
discoverPublicDeclaringClass(this.originalMethod, this.originalMethod.getDeclaringClass());
this.publicDeclaringClass = ReflectionHelper.findPublicDeclaringClass(this.originalMethod);
this.computedPublicDeclaringClass = true;
}
return this.publicDeclaringClass;
}
@Nullable
private Class<?> discoverPublicDeclaringClass(Method method, Class<?> clazz) {
if (Modifier.isPublic(clazz.getModifiers())) {
try {
clazz.getDeclaredMethod(method.getName(), method.getParameterTypes());
return clazz;
}
catch (NoSuchMethodException ex) {
// Continue below...
}
}
if (clazz.getSuperclass() != null) {
return discoverPublicDeclaringClass(method, clazz.getSuperclass());
}
return null;
}
public boolean didArgumentConversionOccur() {
return this.argumentConversionOccurred;
}

55
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 = ClassUtils.getInterfaceMethodIfPossible(method, type);
this.readerCache.put(cacheKey, new InvokerPair(method, typeDescriptor));
Method methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(method, type);
this.readerCache.put(cacheKey, new InvokerPair(methodToInvoke, typeDescriptor, method));
this.typeDescriptorCache.put(cacheKey, typeDescriptor);
return true;
}
@ -171,6 +171,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
if (invoker == null || invoker.member instanceof Method) {
Method method = (Method) (invoker != null ? invoker.member : null);
Method methodToInvoke = method;
if (method == null) {
method = findGetterForProperty(name, type, target);
if (method != null) {
@ -178,15 +179,15 @@ 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 = ClassUtils.getInterfaceMethodIfPossible(method, type);
invoker = new InvokerPair(method, typeDescriptor);
methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(method, type);
invoker = new InvokerPair(methodToInvoke, typeDescriptor, method);
this.readerCache.put(cacheKey, invoker);
}
}
if (method != null) {
if (methodToInvoke != null) {
try {
ReflectionUtils.makeAccessible(method);
Object value = method.invoke(target);
ReflectionUtils.makeAccessible(methodToInvoke);
Object value = methodToInvoke.invoke(target);
return new TypedValue(value, invoker.typeDescriptor.narrow(value));
}
catch (Exception ex) {
@ -532,9 +533,9 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
method = findGetterForProperty(name, type, target);
if (method != null) {
TypeDescriptor typeDescriptor = new TypeDescriptor(new MethodParameter(method, -1));
method = ClassUtils.getInterfaceMethodIfPossible(method, type);
invokerPair = new InvokerPair(method, typeDescriptor);
ReflectionUtils.makeAccessible(method);
Method methodToInvoke = ClassUtils.getInterfaceMethodIfPossible(method, type);
invokerPair = new InvokerPair(methodToInvoke, typeDescriptor, method);
ReflectionUtils.makeAccessible(methodToInvoke);
this.readerCache.put(cacheKey, invokerPair);
}
}
@ -572,8 +573,14 @@ 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) {}
private record InvokerPair(Member member, TypeDescriptor typeDescriptor, @Nullable Method originalMethod) {
InvokerPair(Member member, TypeDescriptor typeDescriptor) {
this(member, typeDescriptor, null);
}
}
private record PropertyCacheKey(Class<?> clazz, String property, boolean targetIsClass)
implements Comparable<PropertyCacheKey> {
@ -606,9 +613,13 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
private final TypeDescriptor typeDescriptor;
@Nullable
private final Method originalMethod;
OptimalPropertyAccessor(InvokerPair invokerPair) {
this.member = invokerPair.member;
this.typeDescriptor = invokerPair.typeDescriptor;
this.originalMethod = invokerPair.originalMethod;
}
@Override
@ -677,8 +688,14 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
@Override
public boolean isCompilable() {
return (Modifier.isPublic(this.member.getModifiers()) &&
Modifier.isPublic(this.member.getDeclaringClass().getModifiers()));
if (Modifier.isPublic(this.member.getModifiers()) &&
Modifier.isPublic(this.member.getDeclaringClass().getModifiers())) {
return true;
}
if (this.originalMethod != null) {
return (ReflectionHelper.findPublicDeclaringClass(this.originalMethod) != null);
}
return false;
}
@Override
@ -693,9 +710,17 @@ 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 = ReflectionHelper.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));
String classDesc = publicDeclaringClass.getName().replace('.', '/');
boolean isStatic = Modifier.isStatic(this.member.getModifiers());
String descriptor = cf.lastDescriptor();
String classDesc = this.member.getDeclaringClass().getName().replace('.', '/');
if (!isStatic) {
if (descriptor == null) {
@ -714,7 +739,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
}
if (this.member instanceof Method method) {
boolean isInterface = method.getDeclaringClass().isInterface();
boolean isInterface = publicDeclaringClass.isInterface();
int opcode = (isStatic ? INVOKESTATIC : isInterface ? INVOKEINTERFACE : INVOKEVIRTUAL);
mv.visitMethodInsn(opcode, classDesc, method.getName(),
CodeFlow.createSignatureDescriptor(method), isInterface);

26
spring-expression/src/test/java/org/springframework/expression/spel/PublicInterface.java Normal file
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.expression.spel;
/**
* This is intentionally a top-level public interface.
*/
public interface PublicInterface {
String getText();
}

40
spring-expression/src/test/java/org/springframework/expression/spel/PublicSuperclass.java Normal file
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.expression.spel;
/**
* This is intentionally a top-level public class.
*/
public class PublicSuperclass {
public int process(int num) {
return num + 1;
}
public int getNumber() {
return 1;
}
public String getMessage() {
return "goodbye";
}
public String greet(String name) {
return "Super, " + name;
}
}

153
spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java
View File

@ -42,6 +42,7 @@ import org.springframework.expression.EvaluationContext;
import org.springframework.expression.Expression;
import org.springframework.expression.TypedValue;
import org.springframework.expression.spel.ast.CompoundExpression;
import org.springframework.expression.spel.ast.InlineList;
import org.springframework.expression.spel.ast.OpLT;
import org.springframework.expression.spel.ast.SpelNodeImpl;
import org.springframework.expression.spel.ast.Ternary;
@ -678,6 +679,158 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
}
@Nested
class PropertyVisibilityTests {
@Test
void privateSubclassOverridesPropertyInPublicInterface() {
expression = parser.parseExpression("text");
PrivateSubclass privateSubclass = new PrivateSubclass();
// Prerequisite: type must not be public for this use case.
assertNotPublic(privateSubclass.getClass());
String result = expression.getValue(context, privateSubclass, String.class);
assertThat(result).isEqualTo("enigma");
assertCanCompile(expression);
result = expression.getValue(context, privateSubclass, String.class);
assertThat(result).isEqualTo("enigma");
}
@Test
void privateSubclassOverridesPropertyInPrivateInterface() {
expression = parser.parseExpression("message");
PrivateSubclass privateSubclass = new PrivateSubclass();
// Prerequisite: type must not be public for this use case.
assertNotPublic(privateSubclass.getClass());
String result = expression.getValue(context, privateSubclass, String.class);
assertThat(result).isEqualTo("hello");
assertCanCompile(expression);
result = expression.getValue(context, privateSubclass, String.class);
assertThat(result).isEqualTo("hello");
}
@Test
void privateSubclassOverridesPropertyInPublicSuperclass() {
expression = parser.parseExpression("number");
PrivateSubclass privateSubclass = new PrivateSubclass();
// Prerequisite: type must not be public for this use case.
assertNotPublic(privateSubclass.getClass());
Integer result = expression.getValue(context, privateSubclass, Integer.class);
assertThat(result).isEqualTo(2);
assertCanCompile(expression);
result = expression.getValue(context, privateSubclass, Integer.class);
assertThat(result).isEqualTo(2);
}
private interface PrivateInterface {
String getMessage();
}
private static class PrivateSubclass extends PublicSuperclass implements PublicInterface, PrivateInterface {
@Override
public int getNumber() {
return 2;
}
@Override
public String getText() {
return "enigma";
}
@Override
public String getMessage() {
return "hello";
}
}
}
@Nested
class MethodVisibilityTests {
/**
* Note that {@link InlineList} creates a list and wraps it via
* {@link Collections#unmodifiableList(List)}, whose concrete type is
* package private.
*/
@Test
void packagePrivateSubclassOverridesMethodInPublicInterface() {
expression = parser.parseExpression("{2021, 2022}");
List<?> inlineList = expression.getValue(List.class);
// Prerequisite: type must not be public for this use case.
assertNotPublic(inlineList.getClass());
expression = parser.parseExpression("{2021, 2022}.contains(2022)");
Boolean result = expression.getValue(context, Boolean.class);
assertThat(result).isTrue();
assertCanCompile(expression);
result = expression.getValue(context, Boolean.class);
assertThat(result).isTrue();
}
@Test
void packagePrivateSubclassOverridesMethodInPrivateInterface() {
expression = parser.parseExpression("greet('Jane')");
PrivateSubclass privateSubclass = new PrivateSubclass();
// Prerequisite: type must not be public for this use case.
assertNotPublic(privateSubclass.getClass());
String result = expression.getValue(context, privateSubclass, String.class);
assertThat(result).isEqualTo("Hello, Jane");
assertCanCompile(expression);
result = expression.getValue(context, privateSubclass, String.class);
assertThat(result).isEqualTo("Hello, Jane");
}
@Test
void privateSubclassOverridesMethodInPublicSuperclass() {
expression = parser.parseExpression("process(2)");
PrivateSubclass privateSubclass = new PrivateSubclass();
// Prerequisite: type must not be public for this use case.
assertNotPublic(privateSubclass.getClass());
Integer result = expression.getValue(context, privateSubclass, Integer.class);
assertThat(result).isEqualTo(2 * 2);
assertCanCompile(expression);
result = expression.getValue(context, privateSubclass, Integer.class);
assertThat(result).isEqualTo(2 * 2);
}
private interface PrivateInterface {
String greet(String name);
}
private static class PrivateSubclass extends PublicSuperclass implements PrivateInterface {
@Override
public int process(int num) {
return num * 2;
}
@Override
public String greet(String name) {
return "Hello, " + name;
}
}
}
@Test
void typeReference() {
expression = parse("T(String)");