diff --git a/spring-web/src/main/java/org/springframework/web/bind/WebExchangeDataBinder.java b/spring-web/src/main/java/org/springframework/web/bind/WebExchangeDataBinder.java index 55c188bd52f..4152444a234 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/WebExchangeDataBinder.java +++ b/spring-web/src/main/java/org/springframework/web/bind/WebExchangeDataBinder.java @@ -24,9 +24,6 @@ import java.util.TreeMap; import reactor.core.publisher.Mono; import org.springframework.beans.MutablePropertyValues; -import org.springframework.core.ResolvableType; -import org.springframework.http.MediaType; -import org.springframework.http.codec.HttpMessageReader; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; @@ -42,11 +39,6 @@ import org.springframework.web.server.ServerWebExchange; */ public class WebExchangeDataBinder extends WebDataBinder { - private static final ResolvableType MULTIVALUE_MAP_TYPE = ResolvableType.forClass(MultiValueMap.class); - - - private HttpMessageReader> formReader = null; - /** * Create a new instance, with default object name. @@ -69,15 +61,6 @@ public class WebExchangeDataBinder extends WebDataBinder { } - public void setFormReader(HttpMessageReader> formReader) { - this.formReader = formReader; - } - - public HttpMessageReader> getFormReader() { - return this.formReader; - } - - /** * Bind the URL query parameters or form data of the body of the given request * to this binder's target. The request body is parsed if the content-type @@ -90,7 +73,8 @@ public class WebExchangeDataBinder extends WebDataBinder { ServerHttpRequest request = exchange.getRequest(); Mono> queryParams = Mono.just(request.getQueryParams()); - Mono> formParams = getFormParams(exchange); + Mono> formParams = + exchange.getFormData().defaultIfEmpty(new LinkedMultiValueMap<>()); return Mono.zip(this::mergeParams, queryParams, formParams) .map(this::getParamsToBind) @@ -102,17 +86,6 @@ public class WebExchangeDataBinder extends WebDataBinder { }); } - private Mono> getFormParams(ServerWebExchange exchange) { - ServerHttpRequest request = exchange.getRequest(); - MediaType contentType = request.getHeaders().getContentType(); - if (this.formReader.canRead(MULTIVALUE_MAP_TYPE, contentType)) { - return this.formReader.readMono(MULTIVALUE_MAP_TYPE, request, Collections.emptyMap()); - } - else { - return Mono.just(new LinkedMultiValueMap<>()); - } - } - @SuppressWarnings("unchecked") private MultiValueMap mergeParams(Object[] paramMaps) { MultiValueMap result = new LinkedMultiValueMap<>(); diff --git a/spring-web/src/main/java/org/springframework/web/server/DefaultServerWebExchangeMutativeBuilder.java b/spring-web/src/main/java/org/springframework/web/server/DefaultServerWebExchangeMutativeBuilder.java index 3e3a1d70a02..8bb292927c3 100644 --- a/spring-web/src/main/java/org/springframework/web/server/DefaultServerWebExchangeMutativeBuilder.java +++ b/spring-web/src/main/java/org/springframework/web/server/DefaultServerWebExchangeMutativeBuilder.java @@ -23,6 +23,7 @@ import reactor.core.publisher.Mono; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.util.Assert; +import org.springframework.util.MultiValueMap; /** * Default implementation of @@ -43,6 +44,8 @@ class DefaultServerWebExchangeMutativeBuilder implements ServerWebExchange.Mutat private Mono session; + private Mono> formData; + public DefaultServerWebExchangeMutativeBuilder(ServerWebExchange delegate) { Assert.notNull(delegate, "'delegate' is required."); @@ -74,10 +77,16 @@ class DefaultServerWebExchangeMutativeBuilder implements ServerWebExchange.Mutat return this; } + @Override + public ServerWebExchange.MutativeBuilder setFormData(Mono> formData) { + this.formData = formData; + return this; + } + @Override public ServerWebExchange build() { - return new MutativeDecorator(this.delegate, - this.request, this.response, this.user, this.session); + return new MutativeDecorator(this.delegate, this.request, this.response, + this.user, this.session, this.formData); } @@ -95,16 +104,19 @@ class DefaultServerWebExchangeMutativeBuilder implements ServerWebExchange.Mutat private final Mono session; + private final Mono> formData; + public MutativeDecorator(ServerWebExchange delegate, ServerHttpRequest request, ServerHttpResponse response, Principal user, - Mono session) { + Mono session, Mono> formData) { super(delegate); this.request = request; this.response = response; this.user = user; this.session = session; + this.formData = formData; } @@ -128,6 +140,11 @@ class DefaultServerWebExchangeMutativeBuilder implements ServerWebExchange.Mutat public Optional getPrincipal() { return (this.user != null ? Optional.of((T) this.user) : getDelegate().getPrincipal()); } + + @Override + public Mono> getFormData() { + return (this.formData != null ? this.formData : getDelegate().getFormData()); + } } } diff --git a/spring-web/src/main/java/org/springframework/web/server/ServerWebExchange.java b/spring-web/src/main/java/org/springframework/web/server/ServerWebExchange.java index 265e5673a35..e219d8412f1 100644 --- a/spring-web/src/main/java/org/springframework/web/server/ServerWebExchange.java +++ b/spring-web/src/main/java/org/springframework/web/server/ServerWebExchange.java @@ -25,6 +25,7 @@ import reactor.core.publisher.Mono; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.util.MultiValueMap; /** * Contract for an HTTP request-response interaction. Provides access to the HTTP @@ -74,6 +75,12 @@ public interface ServerWebExchange { */ Optional getPrincipal(); + /** + * Return the form data from the body of the request or an empty {@code Mono} + * if the Content-Type is not "application/x-www-form-urlencoded". + */ + Mono> getFormData(); + /** * Returns {@code true} if the one of the {@code checkNotModified} methods * in this contract were used and they returned true. @@ -155,6 +162,11 @@ public interface ServerWebExchange { */ MutativeBuilder setSession(Mono session); + /** + * Set the form data. + */ + MutativeBuilder setFormData(Mono> formData); + /** * Build an immutable wrapper that returning the mutated properties. */ diff --git a/spring-web/src/main/java/org/springframework/web/server/ServerWebExchangeDecorator.java b/spring-web/src/main/java/org/springframework/web/server/ServerWebExchangeDecorator.java index aeee3822eb1..2dcd561ad58 100644 --- a/spring-web/src/main/java/org/springframework/web/server/ServerWebExchangeDecorator.java +++ b/spring-web/src/main/java/org/springframework/web/server/ServerWebExchangeDecorator.java @@ -25,6 +25,7 @@ import reactor.core.publisher.Mono; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.util.Assert; +import org.springframework.util.MultiValueMap; /** * A convenient base class for classes that need to wrap another @@ -59,52 +60,57 @@ public class ServerWebExchangeDecorator implements ServerWebExchange { @Override public ServerHttpRequest getRequest() { - return this.getDelegate().getRequest(); + return getDelegate().getRequest(); } @Override public ServerHttpResponse getResponse() { - return this.getDelegate().getResponse(); + return getDelegate().getResponse(); } @Override public Map getAttributes() { - return this.getDelegate().getAttributes(); + return getDelegate().getAttributes(); } @Override public Optional getAttribute(String name) { - return this.getDelegate().getAttribute(name); + return getDelegate().getAttribute(name); } @Override public Mono getSession() { - return this.getDelegate().getSession(); + return getDelegate().getSession(); } @Override public Optional getPrincipal() { - return this.getDelegate().getPrincipal(); + return getDelegate().getPrincipal(); + } + + @Override + public Mono> getFormData() { + return getDelegate().getFormData(); } @Override public boolean isNotModified() { - return this.getDelegate().isNotModified(); + return getDelegate().isNotModified(); } @Override public boolean checkNotModified(Instant lastModified) { - return this.getDelegate().checkNotModified(lastModified); + return getDelegate().checkNotModified(lastModified); } @Override public boolean checkNotModified(String etag) { - return this.getDelegate().checkNotModified(etag); + return getDelegate().checkNotModified(etag); } @Override public boolean checkNotModified(String etag, Instant lastModified) { - return this.getDelegate().checkNotModified(etag, lastModified); + return getDelegate().checkNotModified(etag, lastModified); } diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java b/spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java index a78b68f792f..a7eed8018e7 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java @@ -20,6 +20,7 @@ import java.security.Principal; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; @@ -27,12 +28,17 @@ import java.util.concurrent.ConcurrentHashMap; import reactor.core.publisher.Mono; +import org.springframework.core.ResolvableType; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; +import org.springframework.http.InvalidMediaTypeException; +import org.springframework.http.MediaType; +import org.springframework.http.codec.FormHttpMessageReader; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.util.Assert; +import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebSession; @@ -48,6 +54,11 @@ public class DefaultServerWebExchange implements ServerWebExchange { private static final List SAFE_METHODS = Arrays.asList(HttpMethod.GET, HttpMethod.HEAD); + private static final FormHttpMessageReader FORM_READER = new FormHttpMessageReader(); + + private static final ResolvableType MULTIVALUE_TYPE = + ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class); + private final ServerHttpRequest request; @@ -57,6 +68,8 @@ public class DefaultServerWebExchange implements ServerWebExchange { private final Mono sessionMono; + private final Mono> formDataMono; + private volatile boolean notModified; @@ -66,9 +79,25 @@ public class DefaultServerWebExchange implements ServerWebExchange { Assert.notNull(request, "'request' is required"); Assert.notNull(response, "'response' is required"); Assert.notNull(response, "'sessionManager' is required"); + Assert.notNull(response, "'formReader' is required"); this.request = request; this.response = response; this.sessionMono = sessionManager.getSession(this).cache(); + this.formDataMono = initFormData(request); + } + + private static Mono> initFormData(ServerHttpRequest request) { + MediaType contentType; + try { + contentType = request.getHeaders().getContentType(); + if (MediaType.APPLICATION_FORM_URLENCODED.isCompatibleWith(contentType)) { + return FORM_READER.readMono(MULTIVALUE_TYPE, request, Collections.emptyMap()).cache(); + } + } + catch (InvalidMediaTypeException ex) { + // Ignore + } + return Mono.empty(); } @@ -110,6 +139,11 @@ public class DefaultServerWebExchange implements ServerWebExchange { return Optional.empty(); } + @Override + public Mono> getFormData() { + return this.formDataMono; + } + @Override public boolean isNotModified() { return this.notModified; diff --git a/spring-web/src/test/java/org/springframework/web/bind/WebExchangeDataBinderTests.java b/spring-web/src/test/java/org/springframework/web/bind/WebExchangeDataBinderTests.java index 09ae60b93b2..5cd54066bf1 100644 --- a/spring-web/src/test/java/org/springframework/web/bind/WebExchangeDataBinderTests.java +++ b/spring-web/src/test/java/org/springframework/web/bind/WebExchangeDataBinderTests.java @@ -16,16 +16,15 @@ package org.springframework.web.bind; import java.beans.PropertyEditorSupport; -import java.util.Collections; +import java.io.UnsupportedEncodingException; +import java.net.URLEncoder; +import java.util.Iterator; +import org.jetbrains.annotations.NotNull; import org.junit.Before; import org.junit.Test; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; -import reactor.core.publisher.Mono; -import org.springframework.core.ResolvableType; -import org.springframework.http.codec.HttpMessageReader; +import org.springframework.http.MediaType; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; import org.springframework.tests.sample.beans.ITestBean; @@ -35,14 +34,11 @@ import org.springframework.util.MultiValueMap; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.adapter.DefaultServerWebExchange; import org.springframework.web.server.session.DefaultWebSessionManager; -import org.springframework.web.server.session.WebSessionManager; import static junit.framework.TestCase.assertFalse; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.when; -import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED; /** * Unit tests for {@link WebExchangeDataBinder}. @@ -51,49 +47,31 @@ import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED; */ public class WebExchangeDataBinderTests { - private static final ResolvableType ELEMENT_TYPE = ResolvableType.forClass(MultiValueMap.class); - - private WebExchangeDataBinder binder; private TestBean testBean; - private ServerWebExchange exchange; - - @Mock - private HttpMessageReader> formReader; - - private MultiValueMap formData; + private MockServerHttpRequest request; @Before public void setUp() throws Exception { - MockitoAnnotations.initMocks(this); - this.testBean = new TestBean(); this.binder = new WebExchangeDataBinder(this.testBean, "person"); this.binder.registerCustomEditor(ITestBean.class, new TestBeanPropertyEditor()); - this.binder.setFormReader(this.formReader); - MockServerHttpRequest request = new MockServerHttpRequest(); - MockServerHttpResponse response = new MockServerHttpResponse(); - WebSessionManager sessionManager = new DefaultWebSessionManager(); - this.exchange = new DefaultServerWebExchange(request, response, sessionManager); - - request.getHeaders().setContentType(APPLICATION_FORM_URLENCODED); - - this.formData = new LinkedMultiValueMap<>(); - when(this.formReader.canRead(ELEMENT_TYPE, APPLICATION_FORM_URLENCODED)).thenReturn(true); - when(this.formReader.readMono(ELEMENT_TYPE, request, Collections.emptyMap())) - .thenReturn(Mono.just(formData)); + this.request = new MockServerHttpRequest(); + this.request.getHeaders().setContentType(MediaType.APPLICATION_FORM_URLENCODED); } @Test public void testBindingWithNestedObjectCreation() throws Exception { - this.formData.add("spouse", "someValue"); - this.formData.add("spouse.name", "test"); - this.binder.bind(this.exchange).blockMillis(5000); + MultiValueMap formData = new LinkedMultiValueMap<>(); + formData.add("spouse", "someValue"); + formData.add("spouse.name", "test"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertNotNull(this.testBean.getSpouse()); assertEquals("test", testBean.getSpouse().getName()); @@ -101,13 +79,16 @@ public class WebExchangeDataBinderTests { @Test public void testFieldPrefixCausesFieldReset() throws Exception { - this.formData.add("_postProcessed", "visible"); - this.formData.add("postProcessed", "on"); - this.binder.bind(this.exchange).blockMillis(5000); + MultiValueMap formData = new LinkedMultiValueMap<>(); + formData.add("_postProcessed", "visible"); + formData.add("postProcessed", "on"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertTrue(this.testBean.isPostProcessed()); - this.formData.remove("postProcessed"); - this.binder.bind(this.exchange).blockMillis(5000); + formData.remove("postProcessed"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertFalse(this.testBean.isPostProcessed()); } @@ -115,76 +96,94 @@ public class WebExchangeDataBinderTests { public void testFieldPrefixCausesFieldResetWithIgnoreUnknownFields() throws Exception { this.binder.setIgnoreUnknownFields(false); - this.formData.add("_postProcessed", "visible"); - this.formData.add("postProcessed", "on"); - this.binder.bind(this.exchange).blockMillis(5000); + MultiValueMap formData = new LinkedMultiValueMap<>(); + formData.add("_postProcessed", "visible"); + formData.add("postProcessed", "on"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertTrue(this.testBean.isPostProcessed()); - this.formData.remove("postProcessed"); - this.binder.bind(this.exchange).blockMillis(5000); + formData.remove("postProcessed"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertFalse(this.testBean.isPostProcessed()); } @Test public void testFieldDefault() throws Exception { - this.formData.add("!postProcessed", "off"); - this.formData.add("postProcessed", "on"); - this.binder.bind(this.exchange).blockMillis(5000); + MultiValueMap formData = new LinkedMultiValueMap<>(); + formData.add("!postProcessed", "off"); + formData.add("postProcessed", "on"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertTrue(this.testBean.isPostProcessed()); - this.formData.remove("postProcessed"); - this.binder.bind(this.exchange).blockMillis(5000); + formData.remove("postProcessed"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertFalse(this.testBean.isPostProcessed()); } @Test public void testFieldDefaultPreemptsFieldMarker() throws Exception { - this.formData.add("!postProcessed", "on"); - this.formData.add("_postProcessed", "visible"); - this.formData.add("postProcessed", "on"); - this.binder.bind(this.exchange).blockMillis(5000); + MultiValueMap formData = new LinkedMultiValueMap<>(); + formData.add("!postProcessed", "on"); + formData.add("_postProcessed", "visible"); + formData.add("postProcessed", "on"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertTrue(this.testBean.isPostProcessed()); - this.formData.remove("postProcessed"); - this.binder.bind(this.exchange).blockMillis(5000); + formData.remove("postProcessed"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertTrue(this.testBean.isPostProcessed()); - this.formData.remove("!postProcessed"); - this.binder.bind(this.exchange).blockMillis(5000); + formData.remove("!postProcessed"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertFalse(this.testBean.isPostProcessed()); } @Test public void testFieldDefaultNonBoolean() throws Exception { - this.formData.add("!name", "anonymous"); - this.formData.add("name", "Scott"); - this.binder.bind(this.exchange).blockMillis(5000); + MultiValueMap formData = new LinkedMultiValueMap<>(); + formData.add("!name", "anonymous"); + formData.add("name", "Scott"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertEquals("Scott", this.testBean.getName()); - this.formData.remove("name"); - this.binder.bind(this.exchange).blockMillis(5000); + formData.remove("name"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertEquals("anonymous", this.testBean.getName()); } @Test public void testWithCommaSeparatedStringArray() throws Exception { - this.formData.add("stringArray", "bar"); - this.formData.add("stringArray", "abc"); - this.formData.add("stringArray", "123,def"); - this.binder.bind(this.exchange).blockMillis(5000); + MultiValueMap formData = new LinkedMultiValueMap<>(); + formData.add("stringArray", "bar"); + formData.add("stringArray", "abc"); + formData.add("stringArray", "123,def"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertEquals("Expected all three items to be bound", 3, this.testBean.getStringArray().length); - this.formData.remove("stringArray"); - this.formData.add("stringArray", "123,def"); - this.binder.bind(this.exchange).blockMillis(5000); + formData.remove("stringArray"); + formData.add("stringArray", "123,def"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertEquals("Expected only 1 item to be bound", 1, this.testBean.getStringArray().length); } @Test public void testBindingWithNestedObjectCreationAndWrongOrder() throws Exception { - this.formData.add("spouse.name", "test"); - this.formData.add("spouse", "someValue"); - this.binder.bind(this.exchange).blockMillis(5000); + MultiValueMap formData = new LinkedMultiValueMap<>(); + formData.add("spouse.name", "test"); + formData.add("spouse", "someValue"); + this.request.setBody(generateForm(formData)); + this.binder.bind(createExchange()).blockMillis(5000); assertNotNull(this.testBean.getSpouse()); assertEquals("test", this.testBean.getSpouse().getName()); @@ -192,15 +191,47 @@ public class WebExchangeDataBinderTests { @Test public void testBindingWithQueryParams() throws Exception { - MultiValueMap queryParams = this.exchange.getRequest().getQueryParams(); + MultiValueMap queryParams = createExchange().getRequest().getQueryParams(); queryParams.add("spouse", "someValue"); queryParams.add("spouse.name", "test"); - this.binder.bind(this.exchange).blockMillis(5000); + this.binder.bind(createExchange()).blockMillis(5000); assertNotNull(this.testBean.getSpouse()); assertEquals("test", this.testBean.getSpouse().getName()); } + private String generateForm(MultiValueMap form) { + StringBuilder builder = new StringBuilder(); + try { + for (Iterator names = form.keySet().iterator(); names.hasNext();) { + String name = names.next(); + for (Iterator values = form.get(name).iterator(); values.hasNext();) { + String value = values.next(); + builder.append(URLEncoder.encode(name, "UTF-8")); + if (value != null) { + builder.append('='); + builder.append(URLEncoder.encode(value, "UTF-8")); + if (values.hasNext()) { + builder.append('&'); + } + } + } + if (names.hasNext()) { + builder.append('&'); + } + } + } + catch (UnsupportedEncodingException ex) { + throw new IllegalStateException(ex); + } + return builder.toString(); + } + + @NotNull + private ServerWebExchange createExchange() { + return new DefaultServerWebExchange( + this.request, new MockServerHttpResponse(), new DefaultWebSessionManager()); + } private static class TestBeanPropertyEditor extends PropertyEditorSupport {