From ec0ec7a0d6c0ffcc826062c173797956b22e2284 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Wed, 13 Dec 2023 18:03:30 +0000 Subject: [PATCH] Avoid nested constructor binding if there are no request parameters Closes gh-31821 --- .../validation/DataBinder.java | 19 ++++++- .../validation/DataBinderConstructTests.java | 51 +++++++++++++++++-- .../web/bind/ServletRequestDataBinder.java | 23 +++++++++ .../bind/support/WebExchangeDataBinder.java | 6 +++ .../ExtendedServletRequestDataBinder.java | 11 ++++ 5 files changed, 103 insertions(+), 7 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/validation/DataBinder.java b/spring-context/src/main/java/org/springframework/validation/DataBinder.java index 1c0b4733b1f..12b222d3358 100644 --- a/spring-context/src/main/java/org/springframework/validation/DataBinder.java +++ b/spring-context/src/main/java/org/springframework/validation/DataBinder.java @@ -948,7 +948,7 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter { Class paramType = paramTypes[i]; Object value = valueResolver.resolveValue(paramPath, paramType); - if (value == null && shouldConstructArgument(param)) { + if (value == null && shouldConstructArgument(param) && hasValuesFor(paramPath, valueResolver)) { ResolvableType type = ResolvableType.forMethodParameter(param); args[i] = createObject(type, paramPath + ".", valueResolver); } @@ -1022,6 +1022,15 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter { type.getPackageName().startsWith("java.")); } + private boolean hasValuesFor(String paramPath, ValueResolver resolver) { + for (String name : resolver.getNames()) { + if (name.startsWith(paramPath + ".")) { + return true; + } + } + return false; + } + private void validateConstructorArgument( Class constructorClass, String nestedPath, String name, @Nullable Object value) { @@ -1293,7 +1302,6 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter { * Strategy for {@link #construct constructor binding} to look up the values * to bind to a given constructor parameter. */ - @FunctionalInterface public interface ValueResolver { /** @@ -1305,6 +1313,13 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter { */ @Nullable Object resolveValue(String name, Class type); + + /** + * Return the names of all property values. + * @since 6.1.2 + */ + Set getNames(); + } diff --git a/spring-context/src/test/java/org/springframework/validation/DataBinderConstructTests.java b/spring-context/src/test/java/org/springframework/validation/DataBinderConstructTests.java index 9516eba02c5..8f3dc0af630 100644 --- a/spring-context/src/test/java/org/springframework/validation/DataBinderConstructTests.java +++ b/spring-context/src/test/java/org/springframework/validation/DataBinderConstructTests.java @@ -19,12 +19,14 @@ package org.springframework.validation; import java.beans.ConstructorProperties; import java.util.Map; import java.util.Optional; +import java.util.Set; import jakarta.validation.constraints.NotNull; import org.junit.jupiter.api.Test; import org.springframework.core.ResolvableType; import org.springframework.format.support.DefaultFormattingConversionService; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import static org.assertj.core.api.Assertions.assertThat; @@ -76,6 +78,17 @@ public class DataBinderConstructTests { assertThat(bindingResult.getFieldValue("param3")).isNull(); } + @Test // gh-31821 + void dataClassBindingWithNestedOptionalParameterWithMissingParameter() { + MapValueResolver valueResolver = new MapValueResolver(Map.of("param1", "value1")); + DataBinder binder = initDataBinder(NestedDataClass.class); + binder.construct(valueResolver); + + NestedDataClass dataClass = getTarget(binder); + assertThat(dataClass.param1()).isEqualTo("value1"); + assertThat(dataClass.nestedParam2()).isNull(); + } + @Test void dataClassBindingWithConversionError() { MapValueResolver valueResolver = new MapValueResolver(Map.of("param1", "value1", "param2", "x")); @@ -90,7 +103,7 @@ public class DataBinderConstructTests { } @SuppressWarnings("SameParameterValue") - private static DataBinder initDataBinder(Class targetType) { + private static DataBinder initDataBinder(Class targetType) { DataBinder binder = new DataBinder(null); binder.setTargetType(ResolvableType.forClass(targetType)); binder.setConversionService(new DefaultFormattingConversionService()); @@ -137,17 +150,45 @@ public class DataBinderConstructTests { } + private static class NestedDataClass { + + private final String param1; + + @Nullable + private final DataClass nestedParam2; + + public NestedDataClass(String param1, @Nullable DataClass nestedParam2) { + this.param1 = param1; + this.nestedParam2 = nestedParam2; + } + + public String param1() { + return this.param1; + } + + @Nullable + public DataClass nestedParam2() { + return this.nestedParam2; + } + } + + private static class MapValueResolver implements DataBinder.ValueResolver { - private final Map values; + private final Map map; - private MapValueResolver(Map values) { - this.values = values; + private MapValueResolver(Map map) { + this.map = map; } @Override public Object resolveValue(String name, Class type) { - return values.get(name); + return map.get(name); + } + + @Override + public Set getNames() { + return this.map.keySet(); } } diff --git a/spring-web/src/main/java/org/springframework/web/bind/ServletRequestDataBinder.java b/spring-web/src/main/java/org/springframework/web/bind/ServletRequestDataBinder.java index 921cf2e503f..07ae3555666 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/ServletRequestDataBinder.java +++ b/spring-web/src/main/java/org/springframework/web/bind/ServletRequestDataBinder.java @@ -17,7 +17,10 @@ package org.springframework.web.bind; import java.lang.reflect.Array; +import java.util.Enumeration; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import jakarta.servlet.ServletRequest; import jakarta.servlet.http.HttpServletRequest; @@ -213,6 +216,9 @@ public class ServletRequestDataBinder extends WebDataBinder { private final WebDataBinder dataBinder; + @Nullable + private Set parameterNames; + protected ServletRequestValueResolver(ServletRequest request, WebDataBinder dataBinder) { this.request = request; this.dataBinder = dataBinder; @@ -261,6 +267,23 @@ public class ServletRequestDataBinder extends WebDataBinder { } return null; } + + @Override + public Set getNames() { + if (this.parameterNames == null) { + this.parameterNames = initParameterNames(this.request); + } + return this.parameterNames; + } + + protected Set initParameterNames(ServletRequest request) { + Set set = new LinkedHashSet<>(); + Enumeration enumeration = request.getParameterNames(); + while (enumeration.hasMoreElements()) { + set.add(enumeration.nextElement()); + } + return set; + } } } diff --git a/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java b/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java index 371c31e3e24..e169a2cd6a0 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java +++ b/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java @@ -18,6 +18,7 @@ package org.springframework.web.bind.support; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.TreeMap; import reactor.core.publisher.Mono; @@ -164,6 +165,11 @@ public class WebExchangeDataBinder extends WebDataBinder { public Object resolveValue(String name, Class type) { return this.map.get(name); } + + @Override + public Set getNames() { + return this.map.keySet(); + } } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinder.java index 15fda2594b0..cd19b10eba7 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinder.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinder.java @@ -17,6 +17,7 @@ package org.springframework.web.servlet.mvc.method.annotation; import java.util.Map; +import java.util.Set; import jakarta.servlet.ServletRequest; @@ -121,6 +122,16 @@ public class ExtendedServletRequestDataBinder extends ServletRequestDataBinder { } return value; } + + @Override + protected Set initParameterNames(ServletRequest request) { + Set set = super.initParameterNames(request); + Map uriVars = getUriVars(getRequest()); + if (uriVars != null) { + set.addAll(uriVars.keySet()); + } + return set; + } } }