From ab8de8ec4b004e80979af6e4391e28a8a5da4f8e Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 26 Mar 2026 15:09:44 +0100 Subject: [PATCH] Support Map payloads in FormHttpMessageConverter Prior to this commit, the `FormHttpMessageConverter` would only support `MultiValueMap` payloads for reading and writing. While this can be useful when web forms contain multiple values under the same key, this prevent developers from using a common `Map` type and factory methods like `Map.of(...)`. This commit relaxes constraints in `FormHttpMessageConverter` and ensures that `Map` types are supported for reading and writing URL encoded forms. Note that when reading forms to a `Map`, only the first value for each key will be considered and other values will be dropped if they exist. Closes gh-36408 --- .../pages/web/webmvc/message-converters.adoc | 1 + .../client/match/ContentRequestMatchers.java | 3 +- ...AbstractMockHttpServletRequestBuilder.java | 3 +- .../converter/FormHttpMessageConverter.java | 85 ++++++++++------ .../web/filter/FormContentFilter.java | 3 +- .../FormHttpMessageConverterTests.java | 97 +++++++++++++++---- 6 files changed, 140 insertions(+), 52 deletions(-) diff --git a/framework-docs/modules/ROOT/pages/web/webmvc/message-converters.adoc b/framework-docs/modules/ROOT/pages/web/webmvc/message-converters.adoc index 72726b105bb..c9e48ff2c82 100644 --- a/framework-docs/modules/ROOT/pages/web/webmvc/message-converters.adoc +++ b/framework-docs/modules/ROOT/pages/web/webmvc/message-converters.adoc @@ -26,6 +26,7 @@ By default, this converter supports all text media types(`text/{asterisk}`) and | An `HttpMessageConverter` implementation that can read and write URL encoded forms. By default, this converter reads and writes the `application/x-www-form-urlencoded` media type. Form data is read from and written into a `MultiValueMap`. +`Map` is also supported, but multiple values under the same key will be ignored. | `MultipartHttpMessageConverter` | An `HttpMessageConverter` implementation that can read and write multipart messages. diff --git a/spring-test/src/main/java/org/springframework/test/web/client/match/ContentRequestMatchers.java b/spring-test/src/main/java/org/springframework/test/web/client/match/ContentRequestMatchers.java index 0213190dfac..8b4964aa87a 100644 --- a/spring-test/src/main/java/org/springframework/test/web/client/match/ContentRequestMatchers.java +++ b/spring-test/src/main/java/org/springframework/test/web/client/match/ContentRequestMatchers.java @@ -176,12 +176,13 @@ public class ContentRequestMatchers { return formData(multiValueMap, false); } + @SuppressWarnings("unchecked") private RequestMatcher formData(MultiValueMap expectedMap, boolean containsExactly) { return request -> { MockClientHttpRequest mockRequest = (MockClientHttpRequest) request; MockHttpInputMessage message = new MockHttpInputMessage(mockRequest.getBodyAsBytes()); message.getHeaders().putAll(mockRequest.getHeaders()); - MultiValueMap actualMap = new FormHttpMessageConverter() + MultiValueMap actualMap = (MultiValueMap) new FormHttpMessageConverter() .read(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class), message, null); if (containsExactly) { assertEquals("Form data", expectedMap, actualMap); diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java index e5ba86d373c..fe74c01c312 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java @@ -1022,6 +1022,7 @@ public abstract class AbstractMockHttpServletRequestBuilder parseFormData(MediaType mediaType) { HttpInputMessage message = new HttpInputMessage() { @Override @@ -1038,7 +1039,7 @@ public abstract class AbstractMockHttpServletRequestBuilder) new FormHttpMessageConverter() .read(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class), message, null); } catch (IOException ex) { diff --git a/spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java index 039de5a0823..a24e1aae883 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java @@ -35,6 +35,7 @@ import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.util.ObjectUtils; import org.springframework.util.StreamUtils; import org.springframework.util.StringUtils; @@ -76,11 +77,17 @@ import org.springframework.util.StringUtils; * @since 3.0 * @see org.springframework.util.MultiValueMap */ -public class FormHttpMessageConverter implements SmartHttpMessageConverter> { +public class FormHttpMessageConverter implements SmartHttpMessageConverter { /** The default charset used by the converter. */ public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8; + private static final ResolvableType MULTIVALUE_TYPE = + ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class); + + private static final ResolvableType MAP_TYPE = + ResolvableType.forClassWithGenerics(Map.class, String.class, String.class); + private Charset charset = DEFAULT_CHARSET; @@ -106,28 +113,20 @@ public class FormHttpMessageConverter implements SmartHttpMessageConverter valueClass, @Nullable MediaType mediaType) { - if (!MultiValueMap.class.isAssignableFrom(targetType.toClass()) || - (!targetType.hasUnresolvableGenerics() && - !String.class.isAssignableFrom(targetType.getGeneric(1).toClass()))) { + return canConvert(targetType, mediaType); + } + + private boolean canConvert(ResolvableType targetType, @Nullable MediaType mediaType) { + if (!Map.class.isAssignableFrom(targetType.toClass())) { return false; } for (MediaType supportedMediaType : getSupportedMediaTypes()) { - if (supportedMediaType.isCompatibleWith(mediaType)) { + if (supportedMediaType.includes(mediaType)) { return true; } } @@ -135,7 +134,7 @@ public class FormHttpMessageConverter implements SmartHttpMessageConverter read(ResolvableType type, HttpInputMessage inputMessage, @Nullable Map hints) + public Object read(ResolvableType type, HttpInputMessage inputMessage, @Nullable Map hints) throws IOException, HttpMessageNotReadableException { MediaType contentType = inputMessage.getHeaders().getContentType(); @@ -161,20 +160,31 @@ public class FormHttpMessageConverter implements SmartHttpMessageConverter formData, ResolvableType type, @Nullable MediaType contentType, + public void write(Object data, ResolvableType type, @Nullable MediaType contentType, HttpOutputMessage outputMessage, @Nullable Map hints) throws IOException, HttpMessageNotWritableException { + Assert.isInstanceOf(Map.class, data, "data must be of type Map or MultiValueMap"); + contentType = getFormContentType(contentType); outputMessage.getHeaders().setContentType(contentType); - Charset charset = (contentType.getCharset() != null ? contentType.getCharset() : this.charset); - byte[] bytes = serializeForm(formData, charset).getBytes(charset); + String serializedForm = ""; + if (data instanceof MultiValueMap formData) { + serializedForm = serializeForm((MultiValueMap) formData, charset); + } + else if (data instanceof Map formData) { + serializedForm = serializeForm((Map) formData, charset); + } + byte[] bytes = serializedForm.getBytes(charset); outputMessage.getHeaders().setContentLength(bytes.length); if (outputMessage instanceof StreamingHttpOutputMessage streamingOutputMessage) { @@ -204,6 +214,18 @@ public class FormHttpMessageConverter implements SmartHttpMessageConverter formData, Charset charset) { + StringBuilder builder = new StringBuilder(); + formData.forEach((name, value) -> { + if (name == null) { + Assert.isTrue(ObjectUtils.isEmpty(value), () -> "Null name in form data: " + formData); + return; + } + serializeValue(builder, name, value, charset); + }); + return builder.toString(); + } + protected String serializeForm(MultiValueMap formData, Charset charset) { StringBuilder builder = new StringBuilder(); formData.forEach((name, values) -> { @@ -211,19 +233,20 @@ public class FormHttpMessageConverter implements SmartHttpMessageConverter "Null name in form data: " + formData); return; } - values.forEach(value -> { - if (builder.length() != 0) { - builder.append('&'); - } - builder.append(URLEncoder.encode(name, charset)); - if (value != null) { - builder.append('='); - builder.append(URLEncoder.encode(value, charset)); - } - }); + values.forEach(value -> serializeValue(builder, name, value, charset)); }); - return builder.toString(); } + private void serializeValue(StringBuilder builder, String name, String value, Charset charset) { + if (builder.length() != 0) { + builder.append('&'); + } + builder.append(URLEncoder.encode(name, charset)); + if (value != null) { + builder.append('='); + builder.append(URLEncoder.encode(value, charset)); + } + } + } diff --git a/spring-web/src/main/java/org/springframework/web/filter/FormContentFilter.java b/spring-web/src/main/java/org/springframework/web/filter/FormContentFilter.java index 1d4abc3d52e..a9a1d202427 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/FormContentFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/FormContentFilter.java @@ -94,6 +94,7 @@ public class FormContentFilter extends OncePerRequestFilter { } } + @SuppressWarnings("unchecked") private @Nullable MultiValueMap parseIfNecessary(HttpServletRequest request) throws IOException { if (!shouldParse(request)) { return null; @@ -105,7 +106,7 @@ public class FormContentFilter extends OncePerRequestFilter { return request.getInputStream(); } }; - return this.formConverter.read(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class), inputMessage, null); + return (MultiValueMap) this.formConverter.read(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class), inputMessage, null); } private boolean shouldParse(HttpServletRequest request) { diff --git a/spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java index 030fd7a810b..2e6b89350a0 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java @@ -18,7 +18,9 @@ package org.springframework.http.converter; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.Test; @@ -44,6 +46,7 @@ import static org.springframework.http.MediaType.MULTIPART_RELATED; * @author Rossen Stoyanchev * @author Sam Brannen * @author Sebastien Deleuze + * @author Brian Clozel */ class FormHttpMessageConverterTests { @@ -53,35 +56,57 @@ class FormHttpMessageConverterTests { private static final ResolvableType MULTI_VALUE_MAP = ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class); - private final FormHttpMessageConverter converter = new FormHttpMessageConverter(); + private static final ResolvableType MAP = + ResolvableType.forClassWithGenerics(Map.class, String.class, String.class); + private final FormHttpMessageConverter converter = new FormHttpMessageConverter(); + @Test + void cannotReadToMapsWhenMediaTypeMissing() { + assertCannotRead(MAP, null); + assertCannotRead(MULTI_VALUE_MAP, null); + assertCannotRead(LINKED_MULTI_VALUE_MAP, null); + // without generics + assertCannotRead(ResolvableType.forClass(Map.class), null); + assertCannotRead(ResolvableType.forClass(MultiValueMap.class), null); + assertCannotRead(ResolvableType.forClass(LinkedMultiValueMap.class), null); + } @Test - void canRead() { - assertCanRead(APPLICATION_FORM_URLENCODED); + void canReadToMapTypes() { + assertCanRead(MAP, APPLICATION_FORM_URLENCODED); + assertCanRead(MULTI_VALUE_MAP, APPLICATION_FORM_URLENCODED); assertCanRead(LINKED_MULTI_VALUE_MAP, APPLICATION_FORM_URLENCODED); + // without generics + assertCanRead(ResolvableType.forClass(Map.class), APPLICATION_FORM_URLENCODED); + assertCanRead(ResolvableType.forClass(MultiValueMap.class), APPLICATION_FORM_URLENCODED); assertCanRead(ResolvableType.forClass(LinkedMultiValueMap.class), APPLICATION_FORM_URLENCODED); - - ResolvableType mapStringObject = ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, Object.class); - assertCannotRead(mapStringObject, null); - assertCannotRead(mapStringObject, APPLICATION_FORM_URLENCODED); } @Test void cannotReadMultipart() { // Without custom multipart types supported - asssertCannotReadMultipart(); + assertCannotReadMultipart(); // Should still be the case with custom multipart types supported - asssertCannotReadMultipart(); + assertCannotReadMultipart(); } @Test void canWrite() { assertCanWrite(APPLICATION_FORM_URLENCODED); - assertCanWrite(MediaType.ALL); + assertCannotWrite(MediaType.ALL); + } + + + @Test + void canWriteMapTypes() { + assertCanWrite(MAP, APPLICATION_FORM_URLENCODED); + assertCanWrite(MULTI_VALUE_MAP, APPLICATION_FORM_URLENCODED); assertCanWrite(LINKED_MULTI_VALUE_MAP, APPLICATION_FORM_URLENCODED); + // without generics + assertCanWrite(ResolvableType.forClass(Map.class), APPLICATION_FORM_URLENCODED); + assertCanWrite(ResolvableType.forClass(MultiValueMap.class), APPLICATION_FORM_URLENCODED); assertCanWrite(ResolvableType.forClass(LinkedMultiValueMap.class), APPLICATION_FORM_URLENCODED); } @@ -95,18 +120,38 @@ class FormHttpMessageConverterTests { } @Test - void readForm() throws Exception { + @SuppressWarnings("unchecked") + void readFormAsMultiValueMap() throws Exception { String body = "name+1=value+1&name+2=value+2%2B1&name+2=value+2%2B2&name+3"; MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes(StandardCharsets.ISO_8859_1)); inputMessage.getHeaders().setContentType( new MediaType("application", "x-www-form-urlencoded", StandardCharsets.ISO_8859_1)); - MultiValueMap result = this.converter.read(null, inputMessage); + Object result = this.converter.read(ResolvableType.forClass(MultiValueMap.class), inputMessage, null); - assertThat(result).as("Invalid result").hasSize(3); - assertThat(result.getFirst("name 1")).as("Invalid result").isEqualTo("value 1"); - List values = result.get("name 2"); + assertThat(result).isInstanceOf(MultiValueMap.class); + MultiValueMap form = (MultiValueMap) result; + assertThat(form).as("Invalid result").hasSize(3); + assertThat(form.getFirst("name 1")).as("Invalid result").isEqualTo("value 1"); + List values = form.get("name 2"); assertThat(values).as("Invalid result").containsExactly("value 2+1", "value 2+2"); - assertThat(result.getFirst("name 3")).as("Invalid result").isNull(); + assertThat(form.getFirst("name 3")).as("Invalid result").isNull(); + } + + @Test + @SuppressWarnings("unchecked") + void readFormAsMap() throws Exception { + String body = "name+1=value+1&name+2=value+2&name+3"; + MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes(StandardCharsets.ISO_8859_1)); + inputMessage.getHeaders().setContentType( + new MediaType("application", "x-www-form-urlencoded", StandardCharsets.ISO_8859_1)); + Object result = this.converter.read(ResolvableType.forClass(Map.class), inputMessage, null); + + assertThat(result).isInstanceOf(Map.class); + Map form = (Map) result; + assertThat(form).as("Invalid result").hasSize(3); + assertThat(form.get("name 1")).as("Invalid result").isEqualTo("value 1"); + assertThat(form.get("name 2")).as("Invalid result").isEqualTo("value 2"); + assertThat(form.get("name 3")).as("Invalid result").isNull(); } @Test @@ -131,7 +176,7 @@ class FormHttpMessageConverterTests { } @Test - void writeForm() throws IOException { + void writeFormFromMultiValueMap() throws IOException { MultiValueMap body = new LinkedMultiValueMap<>(); body.set("name 1", "value 1"); body.add("name 2", "value 2+1"); @@ -148,6 +193,22 @@ class FormHttpMessageConverterTests { .as("Invalid content-length").isEqualTo(outputMessage.getBodyAsBytes().length); } + @Test + void writeFormFromMap() throws IOException { + Map body = new HashMap<>(); + body.put("name 1", "value 1"); + body.put("name 2", "value 2"); + MockHttpOutputMessage outputMessage = new MockHttpOutputMessage(); + this.converter.write(body, APPLICATION_FORM_URLENCODED, outputMessage); + + assertThat(outputMessage.getBodyAsString(UTF_8)) + .as("Invalid result").isEqualTo("name+2=value+2&name+1=value+1"); + assertThat(outputMessage.getHeaders().getContentType()) + .as("Invalid content-type").isEqualTo(APPLICATION_FORM_URLENCODED); + assertThat(outputMessage.getHeaders().getContentLength()) + .as("Invalid content-length").isEqualTo(outputMessage.getBodyAsBytes().length); + } + private void assertCanRead(MediaType mediaType) { assertCanRead(MULTI_VALUE_MAP, mediaType); } @@ -156,7 +217,7 @@ class FormHttpMessageConverterTests { assertThat(this.converter.canRead(type, mediaType)).as(type.toClass().getSimpleName() + " : " + mediaType).isTrue(); } - private void asssertCannotReadMultipart() { + private void assertCannotReadMultipart() { assertCannotRead(new MediaType("multipart", "*")); assertCannotRead(MULTIPART_FORM_DATA); assertCannotRead(MULTIPART_MIXED);