From ba56953ea554cf42f81504ee4d31e819900abb51 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 13 Nov 2023 10:24:06 -0800 Subject: [PATCH] Skip ValueObjectBinder if parameter names cannot be discovered Update `ValueObjectBinder` so that it is skipped if parameter names cannot be discovered. This is much more likely as of Since Spring Framework 6.1 as it no longer performs ASM parsing to discover names. Fixes gh-38201 --- .../properties/bind/ValueObjectBinder.java | 41 ++++++++++------- .../bind/ValueObjectBinderTests.java | 45 +++++++++++++++++++ 2 files changed, 71 insertions(+), 15 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java index 7a68330b7b2..5f51e91f341 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java @@ -31,6 +31,8 @@ import java.util.Optional; import kotlin.reflect.KFunction; import kotlin.reflect.KParameter; import kotlin.reflect.jvm.ReflectJvmMapping; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.beans.BeanUtils; import org.springframework.boot.context.properties.source.ConfigurationPropertyName; @@ -43,6 +45,7 @@ import org.springframework.core.ResolvableType; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.convert.ConversionException; +import org.springframework.core.log.LogMessage; import org.springframework.util.Assert; /** @@ -55,6 +58,8 @@ import org.springframework.util.Assert; */ class ValueObjectBinder implements DataObjectBinder { + private static final Log logger = LogFactory.getLog(ValueObjectBinder.class); + private final BindConstructorProvider constructorProvider; ValueObjectBinder(BindConstructorProvider constructorProvider) { @@ -261,15 +266,31 @@ class ValueObjectBinder implements DataObjectBinder { private final List constructorParameters; - private DefaultValueObject(Constructor constructor, ResolvableType type) { + private DefaultValueObject(Constructor constructor, List constructorParameters) { super(constructor); - this.constructorParameters = parseConstructorParameters(constructor, type); + this.constructorParameters = constructorParameters; + } + + @Override + List getConstructorParameters() { + return this.constructorParameters; + } + + @SuppressWarnings("unchecked") + static ValueObject get(Constructor bindConstructor, ResolvableType type) { + String[] names = PARAMETER_NAME_DISCOVERER.getParameterNames(bindConstructor); + if (names == null) { + logger.debug(LogMessage.format( + "Unable to use value object binding with %s as parameter names cannot be discovered", + bindConstructor)); + return null; + } + List constructorParameters = parseConstructorParameters(bindConstructor, type, names); + return new DefaultValueObject<>((Constructor) bindConstructor, constructorParameters); } private static List parseConstructorParameters(Constructor constructor, - ResolvableType type) { - String[] names = PARAMETER_NAME_DISCOVERER.getParameterNames(constructor); - Assert.state(names != null, () -> "Failed to extract parameter names for " + constructor); + ResolvableType type, String[] names) { Parameter[] parameters = constructor.getParameters(); List result = new ArrayList<>(parameters.length); for (int i = 0; i < parameters.length; i++) { @@ -285,16 +306,6 @@ class ValueObjectBinder implements DataObjectBinder { return Collections.unmodifiableList(result); } - @Override - List getConstructorParameters() { - return this.constructorParameters; - } - - @SuppressWarnings("unchecked") - static ValueObject get(Constructor bindConstructor, ResolvableType type) { - return new DefaultValueObject<>((Constructor) bindConstructor, type); - } - } /** diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/ValueObjectBinderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/ValueObjectBinderTests.java index aeffd12431f..a47f30aa0fc 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/ValueObjectBinderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/ValueObjectBinderTests.java @@ -26,11 +26,13 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import com.jayway.jsonpath.JsonPath; import org.junit.jupiter.api.Test; import org.springframework.boot.context.properties.source.ConfigurationPropertyName; import org.springframework.boot.context.properties.source.ConfigurationPropertySource; import org.springframework.boot.context.properties.source.MockConfigurationPropertySource; +import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.ResolvableType; import org.springframework.core.convert.ConversionService; import org.springframework.core.test.tools.SourceFile; @@ -391,6 +393,25 @@ class ValueObjectBinderTests { }); } + @Test // gh-38201 + void bindWithNonExtractableParameterNamesAndNonIterablePropertySource() throws Exception { + verifyJsonPathParametersCannotBeResolved(); + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + source.put("test.value", "test"); + this.sources.add(source.nonIterable()); + Bindable target = Bindable.of(NonExtractableParameterName.class); + NonExtractableParameterName bound = this.binder.bindOrCreate("test", target); + assertThat(bound.getValue()).isEqualTo("test"); + } + + private void verifyJsonPathParametersCannotBeResolved() throws NoSuchFieldException { + Class jsonPathClass = NonExtractableParameterName.class.getDeclaredField("jsonPath").getType(); + Constructor[] constructors = jsonPathClass.getDeclaredConstructors(); + assertThat(constructors).hasSize(1); + constructors[0].setAccessible(true); + assertThat(new DefaultParameterNameDiscoverer().getParameterNames(constructors[0])).isNull(); + } + private void noConfigurationProperty(BindException ex) { assertThat(ex.getProperty()).isNull(); } @@ -845,4 +866,28 @@ class ValueObjectBinderTests { } + static class NonExtractableParameterName { + + private String value; + + private JsonPath jsonPath; + + String getValue() { + return this.value; + } + + void setValue(String value) { + this.value = value; + } + + JsonPath getJsonPath() { + return this.jsonPath; + } + + void setJsonPath(JsonPath jsonPath) { + this.jsonPath = jsonPath; + } + + } + }