From 80faa94afc52bd97b7e8cdfd6e40d2d48a682a18 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Thu, 23 May 2024 10:31:23 +0200 Subject: [PATCH] Support Map in FormHttpMessageConverter This commit changes the FormHttpMessageConverter from a HttpMessageConverter> to a HttpMessageConverter>, so that normal, single-value maps can also be used as form representation, both for reading and writing. Closes gh-32826 --- .../client/match/ContentRequestMatchers.java | 3 +- .../MockHttpServletRequestBuilder.java | 3 +- .../converter/FormHttpMessageConverter.java | 163 +++++++++++------- .../web/filter/FormContentFilter.java | 5 +- .../FormHttpMessageConverterTests.java | 120 ++++++++++++- 5 files changed, 226 insertions(+), 68 deletions(-) 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 a2eb29a0101..faf19a13142 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 @@ -174,12 +174,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().read(null, message); + MultiValueMap actualMap = (MultiValueMap) new FormHttpMessageConverter().read(null, message); if (containsExactly) { assertEquals("Form data", expectedMap, actualMap); } diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java index 433d78e86c2..6ec52e6671e 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java @@ -899,6 +899,7 @@ public class MockHttpServletRequestBuilder } } + @SuppressWarnings("unchecked") private MultiValueMap parseFormData(MediaType mediaType) { HttpInputMessage message = new HttpInputMessage() { @Override @@ -914,7 +915,7 @@ public class MockHttpServletRequestBuilder }; try { - return new FormHttpMessageConverter().read(null, message); + return (MultiValueMap) new FormHttpMessageConverter().read(null, message); } catch (IOException ex) { throw new IllegalStateException("Failed to parse form data in request body", 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 7d325cbade6..8b839f5cd4d 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 @@ -28,6 +28,7 @@ import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.function.BiConsumer; import org.springframework.core.io.Resource; import org.springframework.http.ContentDisposition; @@ -52,9 +53,10 @@ import org.springframework.util.StringUtils; * *

In other words, this converter can read and write the * {@code "application/x-www-form-urlencoded"} media type as + * {@code Map} or as * {@link MultiValueMap MultiValueMap<String, String>}, and it can also * write (but not read) the {@code "multipart/form-data"} and - * {@code "multipart/mixed"} media types as + * {@code "multipart/mixed"} media types as {@code Map} or as * {@link MultiValueMap MultiValueMap<String, Object>}. * *

Multipart Data

@@ -81,7 +83,7 @@ import org.springframework.util.StringUtils; * {@code "multipart/form-data"} content type. * * 
- * RestTemplate restTemplate = new RestTemplate();
+ * RestClient restClient = RestClient.create();
  * // AllEncompassingFormHttpMessageConverter is configured by default
  *
  * MultiValueMap<String, Object> form = new LinkedMultiValueMap<>();
@@ -90,7 +92,12 @@ import org.springframework.util.StringUtils;
  * form.add("field 2", "value 3");
  * form.add("field 3", 4);  // non-String form values supported as of 5.1.4
  *
- * restTemplate.postForLocation("https://example.com/myForm", form);
+ * ResponseEntity<Void> response = restClient.post() + * .uri("https://example.com/myForm") + * .contentType(MULTIPART_FORM_DATA) + * .body(form) + * .retrieve() + * .toBodilessEntity(); * *

The following snippet shows how to do a file upload using the * {@code "multipart/form-data"} content type. @@ -100,7 +107,12 @@ import org.springframework.util.StringUtils; * parts.add("field 1", "value 1"); * parts.add("file", new ClassPathResource("myFile.jpg")); * - * restTemplate.postForLocation("https://example.com/myFileUpload", parts); + * ResponseEntity<Void> response = restClient.post() + * .uri("https://example.com/myForm") + * .contentType(MULTIPART_FORM_DATA) + * .body(parts) + * .retrieve() + * .toBodilessEntity(); * *

The following snippet shows how to do a file upload using the * {@code "multipart/mixed"} content type. @@ -110,40 +122,45 @@ import org.springframework.util.StringUtils; * parts.add("field 1", "value 1"); * parts.add("file", new ClassPathResource("myFile.jpg")); * - * HttpHeaders requestHeaders = new HttpHeaders(); - * requestHeaders.setContentType(MediaType.MULTIPART_MIXED); - * - * restTemplate.postForLocation("https://example.com/myFileUpload", - * new HttpEntity<>(parts, requestHeaders)); + * ResponseEntity<Void> response = restClient.post() + * .uri("https://example.com/myForm") + * .contentType(MULTIPART_MIXED) + * .body(form) + * .retrieve() + * .toBodilessEntity(); * *

The following snippet shows how to do a file upload using the * {@code "multipart/related"} content type. * * 

- * MediaType multipartRelated = new MediaType("multipart", "related");
- *
- * restTemplate.getMessageConverters().stream()
- *     .filter(FormHttpMessageConverter.class::isInstance)
+ * restClient = restClient.mutate()
+ *   .messageConverters(l -> l.stream()
+  *    .filter(FormHttpMessageConverter.class::isInstance)
  *     .map(FormHttpMessageConverter.class::cast)
  *     .findFirst()
  *     .orElseThrow(() -> new IllegalStateException("Failed to find FormHttpMessageConverter"))
- *     .addSupportedMediaTypes(multipartRelated);
+ *     .addSupportedMediaTypes(MULTIPART_RELATED);
  *
  * MultiValueMap<String, Object> parts = new LinkedMultiValueMap<>();
  * parts.add("field 1", "value 1");
  * parts.add("file", new ClassPathResource("myFile.jpg"));
  *
- * HttpHeaders requestHeaders = new HttpHeaders();
- * requestHeaders.setContentType(multipartRelated);
- *
- * restTemplate.postForLocation("https://example.com/myFileUpload",
- *     new HttpEntity<>(parts, requestHeaders));
+ * ResponseEntity<Void> response = restClient.post() + * .uri("https://example.com/myForm") + * .contentType(MULTIPART_RELATED) + * .body(form) + * .retrieve() + * .toBodilessEntity(); * *

Miscellaneous

* *

Some methods in this class were inspired by * {@code org.apache.commons.httpclient.methods.multipart.MultipartRequestEntity}. * + *

As of 6.2, the {@code FormHttpMessageConverter} is parameterized over + * {@code Map}, whereas before it was {@code MultiValueMap}, + * in order to support single-value maps. + * * @author Arjen Poutsma * @author Rossen Stoyanchev * @author Juergen Hoeller @@ -152,7 +169,7 @@ import org.springframework.util.StringUtils; * @see org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter * @see org.springframework.util.MultiValueMap */ -public class FormHttpMessageConverter implements HttpMessageConverter> { +public class FormHttpMessageConverter implements HttpMessageConverter> { /** The default charset used by the converter. */ public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8; @@ -295,7 +312,7 @@ public class FormHttpMessageConverter implements HttpMessageConverter clazz, @Nullable MediaType mediaType) { - if (!MultiValueMap.class.isAssignableFrom(clazz)) { + if (!Map.class.isAssignableFrom(clazz)) { return false; } if (mediaType == null) { @@ -315,7 +332,7 @@ public class FormHttpMessageConverter implements HttpMessageConverter clazz, @Nullable MediaType mediaType) { - if (!MultiValueMap.class.isAssignableFrom(clazz)) { + if (!Map.class.isAssignableFrom(clazz)) { return false; } if (mediaType == null || MediaType.ALL.equals(mediaType)) { @@ -330,59 +347,75 @@ public class FormHttpMessageConverter implements HttpMessageConverter read(@Nullable Class> clazz, + public Map read(@Nullable Class> clazz, HttpInputMessage inputMessage) throws IOException, HttpMessageNotReadableException { MediaType contentType = inputMessage.getHeaders().getContentType(); Charset charset = (contentType != null && contentType.getCharset() != null ? contentType.getCharset() : this.charset); String body = StreamUtils.copyToString(inputMessage.getBody(), charset); - String[] pairs = StringUtils.tokenizeToStringArray(body, "&"); - MultiValueMap result = new LinkedMultiValueMap<>(pairs.length); + + if (clazz == null || MultiValueMap.class.isAssignableFrom(clazz)) { + MultiValueMap result = new LinkedMultiValueMap<>(pairs.length); + readToMap(pairs, charset, result::add); + return result; + } + else { + Map result = CollectionUtils.newLinkedHashMap(pairs.length); + readToMap(pairs, charset, result::putIfAbsent); + return result; + } + } + + private static void readToMap(String[] pairs, Charset charset, BiConsumer addFunction) { for (String pair : pairs) { int idx = pair.indexOf('='); if (idx == -1) { - result.add(URLDecoder.decode(pair, charset), null); + addFunction.accept(URLDecoder.decode(pair, charset), null); } else { String name = URLDecoder.decode(pair.substring(0, idx), charset); String value = URLDecoder.decode(pair.substring(idx + 1), charset); - result.add(name, value); + addFunction.accept(name, value); } } - return result; } @Override @SuppressWarnings("unchecked") - public void write(MultiValueMap map, @Nullable MediaType contentType, HttpOutputMessage outputMessage) + public void write(Map map, @Nullable MediaType contentType, HttpOutputMessage outputMessage) throws IOException, HttpMessageNotWritableException { if (isMultipart(map, contentType)) { - writeMultipart((MultiValueMap) map, contentType, outputMessage); + writeMultipart((Map) map, contentType, outputMessage); } else { - writeForm((MultiValueMap) map, contentType, outputMessage); + writeForm((Map) map, contentType, outputMessage); } } - private boolean isMultipart(MultiValueMap map, @Nullable MediaType contentType) { + private boolean isMultipart(Map map, @Nullable MediaType contentType) { if (contentType != null) { return contentType.getType().equalsIgnoreCase("multipart"); } - for (List values : map.values()) { - for (Object value : values) { - if (value != null && !(value instanceof String)) { - return true; + for (Object value : map.values()) { + if (value instanceof List values) { + for (Object v : values) { + if (v != null && !(v instanceof String)) { + return true; + } } } + else if (value != null && !(value instanceof String)) { + return true; + } } return false; } - private void writeForm(MultiValueMap formData, @Nullable MediaType mediaType, + private void writeForm(Map formData, @Nullable MediaType mediaType, HttpOutputMessage outputMessage) throws IOException { mediaType = getFormContentType(mediaType); @@ -430,30 +463,36 @@ public class FormHttpMessageConverter implements HttpMessageConverter formData, Charset charset) { + protected String serializeForm(Map formData, Charset charset) { StringBuilder builder = new StringBuilder(); - formData.forEach((name, values) -> { + formData.forEach((name, value) -> { + if (value instanceof List values) { if (name == null) { Assert.isTrue(CollectionUtils.isEmpty(values), () -> "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(String.valueOf(value), charset)); - } - }); + values.forEach(v -> appendFormValue(builder, name, v, charset)); + } + else { + appendFormValue(builder, name, value, charset); + } }); - return builder.toString(); } + private static void appendFormValue(StringBuilder builder, String name, @Nullable Object value, Charset charset) { + if (!builder.isEmpty()) { + builder.append('&'); + } + builder.append(URLEncoder.encode(name, charset)); + if (value != null) { + builder.append('='); + builder.append(URLEncoder.encode(String.valueOf(value), charset)); + } + } + private void writeMultipart( - MultiValueMap parts, @Nullable MediaType contentType, HttpOutputMessage outputMessage) + Map parts, @Nullable MediaType contentType, HttpOutputMessage outputMessage) throws IOException { // If the supplied content type is null, fall back to multipart/form-data. @@ -500,16 +539,24 @@ public class FormHttpMessageConverter implements HttpMessageConverter parts, byte[] boundary) throws IOException { - for (Map.Entry> entry : parts.entrySet()) { + private void writeParts(OutputStream os, Map parts, byte[] boundary) throws IOException { + for (Map.Entry entry : parts.entrySet()) { String name = entry.getKey(); - for (Object part : entry.getValue()) { - if (part != null) { - writeBoundary(os, boundary); - writePart(name, getHttpEntity(part), os); - writeNewLine(os); + Object value = entry.getValue(); + if (value instanceof List values) { + for (Object part : values) { + if (part != null) { + writeBoundary(os, boundary); + writePart(name, getHttpEntity(part), os); + writeNewLine(os); + } } } + else if (value != null) { + writeBoundary(os, boundary); + writePart(name, getHttpEntity(value), os); + writeNewLine(os); + } } } 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 bad214fb5d5..5d028416bca 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -95,6 +95,7 @@ public class FormContentFilter extends OncePerRequestFilter { } @Nullable + @SuppressWarnings("unchecked") private MultiValueMap parseIfNecessary(HttpServletRequest request) throws IOException { if (!shouldParse(request)) { return null; @@ -106,7 +107,7 @@ public class FormContentFilter extends OncePerRequestFilter { return request.getInputStream(); } }; - return this.formConverter.read(null, inputMessage); + return (MultiValueMap) this.formConverter.read(null, inputMessage); } 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 98c937bc733..43a8c91ecad 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 @@ -73,6 +73,7 @@ class FormHttpMessageConverterTests { @Test void canRead() { assertCanRead(MultiValueMap.class, null); + assertCanRead(Map.class, null); assertCanRead(APPLICATION_FORM_URLENCODED); assertCannotRead(String.class, null); @@ -118,12 +119,12 @@ class FormHttpMessageConverterTests { } @Test - void readForm() throws Exception { + void readFormMultiValue() 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); + MultiValueMap result = (MultiValueMap) this.converter.read(null, inputMessage); assertThat(result).as("Invalid result").hasSize(3); assertThat(result.getFirst("name 1")).as("Invalid result").isEqualTo("value 1"); @@ -133,7 +134,25 @@ class FormHttpMessageConverterTests { } @Test - void writeForm() throws IOException { + @SuppressWarnings("rawtypes") + void readFormSingleValue() 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)); + Object result = ((HttpMessageConverter) this.converter).read(Map.class, inputMessage); + + assertThat(result).isInstanceOf(Map.class); + assertThat(result).isNotInstanceOf(MultiValueMap.class); + Map map = (Map) result; + assertThat(map).as("Invalid result").hasSize(3); + assertThat(map.get("name 1")).as("Invalid result").isEqualTo("value 1"); + assertThat(map.get("name 2")).as("Invalid result").isEqualTo("value 2+1"); + assertThat(map.get("name 3")).as("Invalid result").isNull(); + } + + @Test + void writeFormMultiValue() throws IOException { MultiValueMap body = new LinkedMultiValueMap<>(); body.set("name 1", "value 1"); body.add("name 2", "value 2+1"); @@ -151,7 +170,24 @@ class FormHttpMessageConverterTests { } @Test - void writeMultipart() throws Exception { + void writeFormSingleValue() throws IOException { + Map body = new LinkedHashMap<>(); + body.put("name 1", "value 1"); + body.put("name 2", "value 2"); + body.put("name 3", null); + MockHttpOutputMessage outputMessage = new MockHttpOutputMessage(); + this.converter.write(body, APPLICATION_FORM_URLENCODED, outputMessage); + + assertThat(outputMessage.getBodyAsString(UTF_8)) + .as("Invalid result").isEqualTo("name+1=value+1&name+2=value+2&name+3"); + assertThat(outputMessage.getHeaders().getContentType()) + .as("Invalid content-type").isEqualTo(APPLICATION_FORM_URLENCODED); + assertThat(outputMessage.getHeaders().getContentLength()) + .as("Invalid content-length").isEqualTo(outputMessage.getBodyAsBytes().length); + } + + @Test + void writeMultipartMultiValue() throws Exception { MultiValueMap parts = new LinkedMultiValueMap<>(); parts.add("name 1", "value 1"); @@ -228,6 +264,78 @@ class FormHttpMessageConverterTests { assertThat(item.getContentType()).isEqualTo("application/json"); } + @Test + void writeMultipartSingleValue() throws Exception { + + Map parts = new LinkedHashMap<>(); + parts.put("name 1", "value 1"); + parts.put("name 2", "value 2"); + parts.put("name 3", null); + + Resource logo = new ClassPathResource("/org/springframework/http/converter/logo.jpg"); + parts.put("logo", logo); + + // SPR-12108 + Resource utf8 = new ClassPathResource("/org/springframework/http/converter/logo.jpg") { + @Override + public String getFilename() { + return "Hall\u00F6le.jpg"; + } + }; + parts.put("utf8", utf8); + + MyBean myBean = new MyBean(); + myBean.setString("foo"); + HttpHeaders entityHeaders = new HttpHeaders(); + entityHeaders.setContentType(APPLICATION_JSON); + HttpEntity entity = new HttpEntity<>(myBean, entityHeaders); + parts.put("json", entity); + + Map parameters = new LinkedHashMap<>(2); + parameters.put("charset", UTF_8.name()); + parameters.put("foo", "bar"); + + MockHttpOutputMessage outputMessage = new MockHttpOutputMessage(); + this.converter.write(parts, new MediaType("multipart", "form-data", parameters), outputMessage); + + final MediaType contentType = outputMessage.getHeaders().getContentType(); + assertThat(contentType.getParameters()).containsKeys("charset", "boundary", "foo"); // gh-21568, gh-25839 + + // see if Commons FileUpload can read what we wrote + FileUpload fileUpload = new FileUpload(); + fileUpload.setFileItemFactory(new DiskFileItemFactory()); + RequestContext requestContext = new MockHttpOutputMessageRequestContext(outputMessage); + List items = fileUpload.parseRequest(requestContext); + assertThat(items).hasSize(5); + FileItem item = items.get(0); + assertThat(item.isFormField()).isTrue(); + assertThat(item.getFieldName()).isEqualTo("name 1"); + assertThat(item.getString()).isEqualTo("value 1"); + + item = items.get(1); + assertThat(item.isFormField()).isTrue(); + assertThat(item.getFieldName()).isEqualTo("name 2"); + assertThat(item.getString()).isEqualTo("value 2"); + + item = items.get(2); + assertThat(item.isFormField()).isFalse(); + assertThat(item.getFieldName()).isEqualTo("logo"); + assertThat(item.getName()).isEqualTo("logo.jpg"); + assertThat(item.getContentType()).isEqualTo("image/jpeg"); + assertThat(item.getSize()).isEqualTo(logo.getFile().length()); + + item = items.get(3); + assertThat(item.isFormField()).isFalse(); + assertThat(item.getFieldName()).isEqualTo("utf8"); + assertThat(item.getName()).isEqualTo("Hall\u00F6le.jpg"); + assertThat(item.getContentType()).isEqualTo("image/jpeg"); + assertThat(item.getSize()).isEqualTo(logo.getFile().length()); + + item = items.get(4); + assertThat(item.getFieldName()).isEqualTo("json"); + assertThat(item.getContentType()).isEqualTo("application/json"); + } + @Test void writeMultipartWithSourceHttpMessageConverter() throws Exception { @@ -401,8 +509,8 @@ class FormHttpMessageConverterTests { } private void assertCanWrite(MediaType mediaType) { - Class clazz = MultiValueMap.class; - assertThat(this.converter.canWrite(clazz, mediaType)).as(clazz.getSimpleName() + " : " + mediaType).isTrue(); + assertThat(this.converter.canWrite(MultiValueMap.class, mediaType)).as("MultiValueMap : " + mediaType).isTrue(); + assertThat(this.converter.canWrite(Map.class, mediaType)).as("Map : " + mediaType).isTrue(); } private void assertCannotWrite(MediaType mediaType) {