From 47f88e123f8bdbf94f4eae068f19307967e867e2 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Sun, 14 Jul 2024 12:01:25 +0200 Subject: [PATCH] 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 --- ...BeanDefinitionPropertiesCodeGenerator.java | 6 +- .../AbstractAutowireCapableBeanFactory.java | 2 +- .../support/DisposableBeanAdapter.java | 6 +- ...efinitionPropertiesCodeGeneratorTests.java | 4 +- .../org/springframework/util/ClassUtils.java | 106 ++++++- .../springframework/util/ClassUtilsTests.java | 278 ++++++++++++++++++ .../springframework/util/PublicInterface.java | 26 ++ .../util/PublicSuperclass.java | 40 +++ .../expression/spel/CodeFlow.java | 72 ----- .../expression/spel/ast/MethodReference.java | 12 +- .../spel/support/ReflectiveIndexAccessor.java | 9 +- .../support/ReflectiveMethodExecutor.java | 36 +-- .../support/ReflectivePropertyAccessor.java | 51 +--- .../spel/MethodInvocationTests.java | 24 +- .../expression/spel/PropertyAccessTests.java | 18 +- 15 files changed, 519 insertions(+), 171 deletions(-) create mode 100644 spring-core/src/test/java/org/springframework/util/PublicInterface.java create mode 100644 spring-core/src/test/java/org/springframework/util/PublicSuperclass.java diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java index 91cd2a14b98..49d7d8083db 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGenerator.java @@ -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); } } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index 3a460f41a20..d1ade588ec1 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -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); diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java index 14198c4b2f1..f95fd951e3f 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java @@ -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); } } } diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java index 7a9f52e0e59..2fc485a5abd 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionPropertiesCodeGeneratorTests.java @@ -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(); } diff --git a/spring-core/src/main/java/org/springframework/util/ClassUtils.java b/spring-core/src/main/java/org/springframework/util/ClassUtils.java index e191a0552dd..cab2e454bb4 100644 --- a/spring-core/src/main/java/org/springframework/util/ClassUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ClassUtils.java @@ -135,10 +135,17 @@ public abstract class ClassUtils { private static final Set> 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 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 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. + *

If the supplied method is {@code public} and declared in a {@code 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 {@code public} and declared in a + * {@code public} type. + *

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. + *

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 + * — 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; } /** diff --git a/spring-core/src/test/java/org/springframework/util/ClassUtilsTests.java b/spring-core/src/test/java/org/springframework/util/ClassUtilsTests.java index dd834e75dfa..bfc10a2d19a 100644 --- a/spring-core/src/test/java/org/springframework/util/ClassUtilsTests.java +++ b/spring-core/src/test/java/org/springframework/util/ClassUtilsTests.java @@ -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 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 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 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 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, diff --git a/spring-core/src/test/java/org/springframework/util/PublicInterface.java b/spring-core/src/test/java/org/springframework/util/PublicInterface.java new file mode 100644 index 00000000000..7812b38459c --- /dev/null +++ b/spring-core/src/test/java/org/springframework/util/PublicInterface.java @@ -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(); + +} diff --git a/spring-core/src/test/java/org/springframework/util/PublicSuperclass.java b/spring-core/src/test/java/org/springframework/util/PublicSuperclass.java new file mode 100644 index 00000000000..e976d9b26ba --- /dev/null +++ b/spring-core/src/test/java/org/springframework/util/PublicSuperclass.java @@ -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; + } + +} diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java b/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java index d9d3bbcbbfa..7a3d4489467 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java @@ -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> 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. - *

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 diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java index 5006a03a493..e8d15e5b053 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java @@ -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); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveIndexAccessor.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveIndexAccessor.java index abf0ec7505a..601f2792c29 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveIndexAccessor.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveIndexAccessor.java @@ -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('.', '/'); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java index f93bea65503..b66312328f9 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodExecutor.java @@ -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}. - *

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}. + *

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; } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java index 52ca54de86f..b7b9644a8e1 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java @@ -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. *

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. *

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. - *

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()); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java index 3da7456189d..e2f8f79b4aa 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java @@ -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 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(); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java index d763088b7ce..24850844da5 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java @@ -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();