Browse Source

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
pull/36549/head
Brian Clozel 4 days ago
parent
commit
ab8de8ec4b
  1. 1
      framework-docs/modules/ROOT/pages/web/webmvc/message-converters.adoc
  2. 3
      spring-test/src/main/java/org/springframework/test/web/client/match/ContentRequestMatchers.java
  3. 3
      spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java
  4. 85
      spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java
  5. 3
      spring-web/src/main/java/org/springframework/web/filter/FormContentFilter.java
  6. 97
      spring-web/src/test/java/org/springframework/http/converter/FormHttpMessageConverterTests.java

1
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. | 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. 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<String, String>`. Form data is read from and written into a `MultiValueMap<String, String>`.
`Map<String, String>` is also supported, but multiple values under the same key will be ignored.
| `MultipartHttpMessageConverter` | `MultipartHttpMessageConverter`
| An `HttpMessageConverter` implementation that can read and write multipart messages. | An `HttpMessageConverter` implementation that can read and write multipart messages.

3
spring-test/src/main/java/org/springframework/test/web/client/match/ContentRequestMatchers.java

@ -176,12 +176,13 @@ public class ContentRequestMatchers {
return formData(multiValueMap, false); return formData(multiValueMap, false);
} }
@SuppressWarnings("unchecked")
private RequestMatcher formData(MultiValueMap<String, String> expectedMap, boolean containsExactly) { private RequestMatcher formData(MultiValueMap<String, String> expectedMap, boolean containsExactly) {
return request -> { return request -> {
MockClientHttpRequest mockRequest = (MockClientHttpRequest) request; MockClientHttpRequest mockRequest = (MockClientHttpRequest) request;
MockHttpInputMessage message = new MockHttpInputMessage(mockRequest.getBodyAsBytes()); MockHttpInputMessage message = new MockHttpInputMessage(mockRequest.getBodyAsBytes());
message.getHeaders().putAll(mockRequest.getHeaders()); message.getHeaders().putAll(mockRequest.getHeaders());
MultiValueMap<String, String> actualMap = new FormHttpMessageConverter() MultiValueMap<String, String> actualMap = (MultiValueMap<String, String>) new FormHttpMessageConverter()
.read(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class), message, null); .read(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class), message, null);
if (containsExactly) { if (containsExactly) {
assertEquals("Form data", expectedMap, actualMap); assertEquals("Form data", expectedMap, actualMap);

3
spring-test/src/main/java/org/springframework/test/web/servlet/request/AbstractMockHttpServletRequestBuilder.java

@ -1022,6 +1022,7 @@ public abstract class AbstractMockHttpServletRequestBuilder<B extends AbstractMo
} }
} }
@SuppressWarnings("unchecked")
private MultiValueMap<String, String> parseFormData(MediaType mediaType) { private MultiValueMap<String, String> parseFormData(MediaType mediaType) {
HttpInputMessage message = new HttpInputMessage() { HttpInputMessage message = new HttpInputMessage() {
@Override @Override
@ -1038,7 +1039,7 @@ public abstract class AbstractMockHttpServletRequestBuilder<B extends AbstractMo
}; };
try { try {
return new FormHttpMessageConverter() return (MultiValueMap<String, String>) new FormHttpMessageConverter()
.read(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class), message, null); .read(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class), message, null);
} }
catch (IOException ex) { catch (IOException ex) {

85
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.CollectionUtils;
import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap; import org.springframework.util.MultiValueMap;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StreamUtils; import org.springframework.util.StreamUtils;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
@ -76,11 +77,17 @@ import org.springframework.util.StringUtils;
* @since 3.0 * @since 3.0
* @see org.springframework.util.MultiValueMap * @see org.springframework.util.MultiValueMap
*/ */
public class FormHttpMessageConverter implements SmartHttpMessageConverter<MultiValueMap<String, String>> { public class FormHttpMessageConverter implements SmartHttpMessageConverter<Object> {
/** The default charset used by the converter. */ /** The default charset used by the converter. */
public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8; 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; private Charset charset = DEFAULT_CHARSET;
@ -106,28 +113,20 @@ public class FormHttpMessageConverter implements SmartHttpMessageConverter<Multi
@Override @Override
public boolean canRead(ResolvableType type, @Nullable MediaType mediaType) { public boolean canRead(ResolvableType type, @Nullable MediaType mediaType) {
if (!MultiValueMap.class.isAssignableFrom(type.toClass()) || return canConvert(type, mediaType);
(!type.hasUnresolvableGenerics() &&
!String.class.isAssignableFrom(type.getGeneric(1).toClass()))) {
return false;
}
for (MediaType supportedMediaType : getSupportedMediaTypes()) {
if (supportedMediaType.includes(mediaType)) {
return true;
}
}
return false;
} }
@Override @Override
public boolean canWrite(ResolvableType targetType, Class<?> valueClass, @Nullable MediaType mediaType) { public boolean canWrite(ResolvableType targetType, Class<?> valueClass, @Nullable MediaType mediaType) {
if (!MultiValueMap.class.isAssignableFrom(targetType.toClass()) || return canConvert(targetType, mediaType);
(!targetType.hasUnresolvableGenerics() && }
!String.class.isAssignableFrom(targetType.getGeneric(1).toClass()))) {
private boolean canConvert(ResolvableType targetType, @Nullable MediaType mediaType) {
if (!Map.class.isAssignableFrom(targetType.toClass())) {
return false; return false;
} }
for (MediaType supportedMediaType : getSupportedMediaTypes()) { for (MediaType supportedMediaType : getSupportedMediaTypes()) {
if (supportedMediaType.isCompatibleWith(mediaType)) { if (supportedMediaType.includes(mediaType)) {
return true; return true;
} }
} }
@ -135,7 +134,7 @@ public class FormHttpMessageConverter implements SmartHttpMessageConverter<Multi
} }
@Override @Override
public MultiValueMap<String, String> read(ResolvableType type, HttpInputMessage inputMessage, @Nullable Map<String, Object> hints) public Object read(ResolvableType type, HttpInputMessage inputMessage, @Nullable Map<String, Object> hints)
throws IOException, HttpMessageNotReadableException { throws IOException, HttpMessageNotReadableException {
MediaType contentType = inputMessage.getHeaders().getContentType(); MediaType contentType = inputMessage.getHeaders().getContentType();
@ -161,20 +160,31 @@ public class FormHttpMessageConverter implements SmartHttpMessageConverter<Multi
catch (IllegalArgumentException ex) { catch (IllegalArgumentException ex) {
throw new HttpMessageNotReadableException("Could not decode HTTP form payload", ex, inputMessage); throw new HttpMessageNotReadableException("Could not decode HTTP form payload", ex, inputMessage);
} }
if (!MultiValueMap.class.isAssignableFrom(type.toClass())) {
return result.asSingleValueMap();
}
return result; return result;
} }
@Override @Override
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
public void write(MultiValueMap<String, String> formData, ResolvableType type, @Nullable MediaType contentType, public void write(Object data, ResolvableType type, @Nullable MediaType contentType,
HttpOutputMessage outputMessage, @Nullable Map<String, Object> hints) throws IOException, HttpMessageNotWritableException { HttpOutputMessage outputMessage, @Nullable Map<String, Object> hints) throws IOException, HttpMessageNotWritableException {
Assert.isInstanceOf(Map.class, data, "data must be of type Map or MultiValueMap");
contentType = getFormContentType(contentType); contentType = getFormContentType(contentType);
outputMessage.getHeaders().setContentType(contentType); outputMessage.getHeaders().setContentType(contentType);
Charset charset = (contentType.getCharset() != null ? contentType.getCharset() : this.charset); 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<String, String>) formData, charset);
}
else if (data instanceof Map<?,?> formData) {
serializedForm = serializeForm((Map<String, String>) formData, charset);
}
byte[] bytes = serializedForm.getBytes(charset);
outputMessage.getHeaders().setContentLength(bytes.length); outputMessage.getHeaders().setContentLength(bytes.length);
if (outputMessage instanceof StreamingHttpOutputMessage streamingOutputMessage) { if (outputMessage instanceof StreamingHttpOutputMessage streamingOutputMessage) {
@ -204,6 +214,18 @@ public class FormHttpMessageConverter implements SmartHttpMessageConverter<Multi
return contentType; return contentType;
} }
protected String serializeForm(Map<String, String> 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<String, String> formData, Charset charset) { protected String serializeForm(MultiValueMap<String, String> formData, Charset charset) {
StringBuilder builder = new StringBuilder(); StringBuilder builder = new StringBuilder();
formData.forEach((name, values) -> { formData.forEach((name, values) -> {
@ -211,19 +233,20 @@ public class FormHttpMessageConverter implements SmartHttpMessageConverter<Multi
Assert.isTrue(CollectionUtils.isEmpty(values), () -> "Null name in form data: " + formData); Assert.isTrue(CollectionUtils.isEmpty(values), () -> "Null name in form data: " + formData);
return; return;
} }
values.forEach(value -> { values.forEach(value -> serializeValue(builder, name, value, charset));
if (builder.length() != 0) {
builder.append('&');
}
builder.append(URLEncoder.encode(name, charset));
if (value != null) {
builder.append('=');
builder.append(URLEncoder.encode(value, charset));
}
});
}); });
return builder.toString(); 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));
}
}
} }

3
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<String, String> parseIfNecessary(HttpServletRequest request) throws IOException { private @Nullable MultiValueMap<String, String> parseIfNecessary(HttpServletRequest request) throws IOException {
if (!shouldParse(request)) { if (!shouldParse(request)) {
return null; return null;
@ -105,7 +106,7 @@ public class FormContentFilter extends OncePerRequestFilter {
return request.getInputStream(); return request.getInputStream();
} }
}; };
return this.formConverter.read(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class), inputMessage, null); return (MultiValueMap<String, String>) this.formConverter.read(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class), inputMessage, null);
} }
private boolean shouldParse(HttpServletRequest request) { private boolean shouldParse(HttpServletRequest request) {

97
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.io.IOException;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
@ -44,6 +46,7 @@ import static org.springframework.http.MediaType.MULTIPART_RELATED;
* @author Rossen Stoyanchev * @author Rossen Stoyanchev
* @author Sam Brannen * @author Sam Brannen
* @author Sebastien Deleuze * @author Sebastien Deleuze
* @author Brian Clozel
*/ */
class FormHttpMessageConverterTests { class FormHttpMessageConverterTests {
@ -53,35 +56,57 @@ class FormHttpMessageConverterTests {
private static final ResolvableType MULTI_VALUE_MAP = private static final ResolvableType MULTI_VALUE_MAP =
ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class); 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 @Test
void canRead() { void canReadToMapTypes() {
assertCanRead(APPLICATION_FORM_URLENCODED); assertCanRead(MAP, APPLICATION_FORM_URLENCODED);
assertCanRead(MULTI_VALUE_MAP, APPLICATION_FORM_URLENCODED);
assertCanRead(LINKED_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); 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 @Test
void cannotReadMultipart() { void cannotReadMultipart() {
// Without custom multipart types supported // Without custom multipart types supported
asssertCannotReadMultipart(); assertCannotReadMultipart();
// Should still be the case with custom multipart types supported // Should still be the case with custom multipart types supported
asssertCannotReadMultipart(); assertCannotReadMultipart();
} }
@Test @Test
void canWrite() { void canWrite() {
assertCanWrite(APPLICATION_FORM_URLENCODED); 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); 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); assertCanWrite(ResolvableType.forClass(LinkedMultiValueMap.class), APPLICATION_FORM_URLENCODED);
} }
@ -95,18 +120,38 @@ class FormHttpMessageConverterTests {
} }
@Test @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"; 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)); MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes(StandardCharsets.ISO_8859_1));
inputMessage.getHeaders().setContentType( inputMessage.getHeaders().setContentType(
new MediaType("application", "x-www-form-urlencoded", StandardCharsets.ISO_8859_1)); new MediaType("application", "x-www-form-urlencoded", StandardCharsets.ISO_8859_1));
MultiValueMap<String, String> 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).isInstanceOf(MultiValueMap.class);
assertThat(result.getFirst("name 1")).as("Invalid result").isEqualTo("value 1"); MultiValueMap<String, String> form = (MultiValueMap<String, String>) result;
List<String> values = result.get("name 2"); assertThat(form).as("Invalid result").hasSize(3);
assertThat(form.getFirst("name 1")).as("Invalid result").isEqualTo("value 1");
List<String> values = form.get("name 2");
assertThat(values).as("Invalid result").containsExactly("value 2+1", "value 2+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<String, String> form = (Map<String, String>) 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 @Test
@ -131,7 +176,7 @@ class FormHttpMessageConverterTests {
} }
@Test @Test
void writeForm() throws IOException { void writeFormFromMultiValueMap() throws IOException {
MultiValueMap<String, String> body = new LinkedMultiValueMap<>(); MultiValueMap<String, String> body = new LinkedMultiValueMap<>();
body.set("name 1", "value 1"); body.set("name 1", "value 1");
body.add("name 2", "value 2+1"); body.add("name 2", "value 2+1");
@ -148,6 +193,22 @@ class FormHttpMessageConverterTests {
.as("Invalid content-length").isEqualTo(outputMessage.getBodyAsBytes().length); .as("Invalid content-length").isEqualTo(outputMessage.getBodyAsBytes().length);
} }
@Test
void writeFormFromMap() throws IOException {
Map<String, String> 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) { private void assertCanRead(MediaType mediaType) {
assertCanRead(MULTI_VALUE_MAP, mediaType); assertCanRead(MULTI_VALUE_MAP, mediaType);
} }
@ -156,7 +217,7 @@ class FormHttpMessageConverterTests {
assertThat(this.converter.canRead(type, mediaType)).as(type.toClass().getSimpleName() + " : " + mediaType).isTrue(); assertThat(this.converter.canRead(type, mediaType)).as(type.toClass().getSimpleName() + " : " + mediaType).isTrue();
} }
private void asssertCannotReadMultipart() { private void assertCannotReadMultipart() {
assertCannotRead(new MediaType("multipart", "*")); assertCannotRead(new MediaType("multipart", "*"));
assertCannotRead(MULTIPART_FORM_DATA); assertCannotRead(MULTIPART_FORM_DATA);
assertCannotRead(MULTIPART_MIXED); assertCannotRead(MULTIPART_MIXED);

Loading…
Cancel
Save