diff --git a/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java index ffee0fb851..fa4ed289c3 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.Map; import org.reactivestreams.Publisher; -import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import org.springframework.core.ResolvableType; @@ -39,12 +38,24 @@ import org.springframework.util.Assert; import org.springframework.util.MultiValueMap; /** - * Implementation of an {@link HttpMessageWriter} to write HTML form data, i.e. - * response body with media type {@code "application/x-www-form-urlencoded"}. + * {@link HttpMessageWriter} for writing a {@code MultiValueMap} + * as HTML form data, i.e. {@code "application/x-www-form-urlencoded"}, to the + * body of a request. + * + *

Note that unless the media type is explicitly set to + * {@link MediaType#APPLICATION_FORM_URLENCODED}, the {@link #canWrite} method + * will need generic type information to confirm the target map has String values. + * This is because a MultiValueMap with non-String values can be used to write + * multipart requests. + * + *

To support both form data and multipart requests, consider using + * {@link org.springframework.http.codec.multipart.MultipartHttpMessageWriter} + * configured with this writer as the fallback for writing plain form data. * * @author Sebastien Deleuze * @author Rossen Stoyanchev * @since 5.0 + * @see org.springframework.http.codec.multipart.MultipartHttpMessageWriter */ public class FormHttpMessageWriter implements HttpMessageWriter> { @@ -53,6 +64,9 @@ public class FormHttpMessageWriter implements HttpMessageWriter MEDIA_TYPES = + Collections.singletonList(MediaType.APPLICATION_FORM_URLENCODED); + private Charset defaultCharset = DEFAULT_CHARSET; @@ -75,12 +89,27 @@ public class FormHttpMessageWriter implements HttpMessageWriter getWritableMediaTypes() { + return MEDIA_TYPES; + } + + @Override public boolean canWrite(ResolvableType elementType, @Nullable MediaType mediaType) { - return (MULTIVALUE_TYPE.isAssignableFrom(elementType) || - (elementType.hasUnresolvableGenerics() && - MultiValueMap.class.isAssignableFrom(elementType.resolve(Object.class)))) && - (mediaType == null || MediaType.APPLICATION_FORM_URLENCODED.isCompatibleWith(mediaType)); + Class rawClass = elementType.getRawClass(); + if (rawClass == null || !MultiValueMap.class.isAssignableFrom(rawClass)) { + return false; + } + if (MediaType.APPLICATION_FORM_URLENCODED.isCompatibleWith(mediaType)) { + // Optimistically, any MultiValueMap with or without generics + return true; + } + if (mediaType == null) { + // Only String-based MultiValueMap + return MULTIVALUE_TYPE.isAssignableFrom(elementType); + } + return false; } @Override @@ -96,11 +125,8 @@ public class FormHttpMessageWriter implements HttpMessageWriter generateForm(form, charset)) - .flatMap(value -> { + return Mono.from(inputStream).flatMap(form -> { + String value = serializeForm(form, charset); ByteBuffer byteBuffer = charset.encode(value); DataBuffer buffer = message.bufferFactory().wrap(byteBuffer); message.getHeaders().setContentLength(byteBuffer.remaining()); @@ -118,17 +144,20 @@ public class FormHttpMessageWriter implements HttpMessageWriter form, Charset charset) { + private String serializeForm(MultiValueMap form, Charset charset) { 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(); + for (Iterator values = form.get(name).iterator(); values.hasNext();) { + Object rawValue = values.next(); builder.append(URLEncoder.encode(name, charset.name())); - if (value != null) { + if (rawValue != null) { builder.append('='); - builder.append(URLEncoder.encode(value, charset.name())); + Assert.isInstanceOf(String.class, rawValue, + "FormHttpMessageWriter supports String values only. " + + "Use MultipartHttpMessageWriter for multipart requests."); + builder.append(URLEncoder.encode((String) rawValue, charset.name())); if (values.hasNext()) { builder.append('&'); } @@ -145,9 +174,4 @@ public class FormHttpMessageWriter implements HttpMessageWriter getWritableMediaTypes() { - return Collections.singletonList(MediaType.APPLICATION_FORM_URLENCODED); - } - } diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java index ec73736ffb..03008459a6 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java @@ -18,6 +18,7 @@ package org.springframework.http.codec.multipart; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -44,6 +45,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.ReactiveHttpOutputMessage; import org.springframework.http.codec.EncoderHttpMessageWriter; +import org.springframework.http.codec.FormHttpMessageWriter; import org.springframework.http.codec.HttpMessageWriter; import org.springframework.http.codec.ResourceHttpMessageWriter; import org.springframework.lang.Nullable; @@ -52,37 +54,82 @@ import org.springframework.util.MimeTypeUtils; import org.springframework.util.MultiValueMap; /** - * {@code HttpMessageWriter} for {@code "multipart/form-data"} requests. + * {@link HttpMessageWriter} for writing a {@code MultiValueMap} + * as multipart form data, i.e. {@code "multipart/form-data"}, to the body + * of a request. * - *

This writer delegates to other message writers to write the respective - * parts. By default basic writers are registered for {@code String}, and - * {@code Resources}. These can be overridden through the provided constructors. + *

The serialization of individual parts is delegated to other writers. + * By default only {@link String} and {@link Resource} parts are supported but + * you can configure others through a constructor argument. + * + *

This writer can be configured with a {@link FormHttpMessageWriter} to + * delegate to. It is the preferred way of supporting both form data and + * multipart data (as opposed to registering each writer separately) so that + * when the {@link MediaType} is not specified and generics are not present on + * the target element type, we can inspect the values in the actual map and + * decide whether to write plain form data (String values only) or otherwise. * * @author Sebastien Deleuze * @author Rossen Stoyanchev * @since 5.0 + * @see FormHttpMessageWriter */ public class MultipartHttpMessageWriter implements HttpMessageWriter> { public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8; - private final DataBufferFactory bufferFactory = new DefaultDataBufferFactory(); - private final List> partWriters; + @Nullable + private final HttpMessageWriter> formWriter; + private Charset charset = DEFAULT_CHARSET; + private final List supportedMediaTypes; + private final DataBufferFactory bufferFactory = new DefaultDataBufferFactory(); + + + /** + * Constructor with a default list of part writers (String and Resource). + */ public MultipartHttpMessageWriter() { - this.partWriters = Arrays.asList( + this(Arrays.asList( new EncoderHttpMessageWriter<>(CharSequenceEncoder.textPlainOnly()), new ResourceHttpMessageWriter() - ); + )); } + /** + * Constructor with explicit list of writers for serializing parts. + */ public MultipartHttpMessageWriter(List> partWriters) { + this(partWriters, new FormHttpMessageWriter()); + } + + /** + * Constructor with explicit list of writers for serializing parts and a + * writer for plain form data to fall back when no media type is specified + * and the actual map consists of String values only. + * @param partWriters the writers for serializing parts + * @param formWriter the fallback writer for form data, {@code null} by default + */ + public MultipartHttpMessageWriter(List> partWriters, + @Nullable HttpMessageWriter> formWriter) { + this.partWriters = partWriters; + this.formWriter = formWriter; + this.supportedMediaTypes = initMediaTypes(formWriter); + } + + private static List initMediaTypes(@Nullable HttpMessageWriter formWriter) { + List result = new ArrayList<>(); + result.add(MediaType.MULTIPART_FORM_DATA); + if (formWriter != null) { + result.addAll(formWriter.getWritableMediaTypes()); + } + return Collections.unmodifiableList(result); } @@ -106,14 +153,15 @@ public class MultipartHttpMessageWriter implements HttpMessageWriter getWritableMediaTypes() { - return Collections.singletonList(MediaType.MULTIPART_FORM_DATA); + return this.supportedMediaTypes; } @Override public boolean canWrite(ResolvableType elementType, @Nullable MediaType mediaType) { Class rawClass = elementType.getRawClass(); - return (rawClass != null && MultiValueMap.class.isAssignableFrom(rawClass) && - (mediaType == null || MediaType.MULTIPART_FORM_DATA.isCompatibleWith(mediaType))); + return rawClass != null && MultiValueMap.class.isAssignableFrom(rawClass) && + (mediaType == null || + this.supportedMediaTypes.stream().anyMatch(m -> m.isCompatibleWith(mediaType))); } @Override @@ -121,19 +169,47 @@ public class MultipartHttpMessageWriter implements HttpMessageWriter hints) { + return Mono.from(inputStream).flatMap(map -> { + if (this.formWriter == null || isMultipart(map, mediaType)) { + return writeMultipart(map, outputMessage); + } + else { + @SuppressWarnings("unchecked") + MultiValueMap formData = (MultiValueMap) map; + return this.formWriter.write(Mono.just(formData), elementType, mediaType, outputMessage, hints); + } + + }); + } + + private boolean isMultipart(MultiValueMap map, @Nullable MediaType contentType) { + if (contentType != null) { + return MediaType.MULTIPART_FORM_DATA.includes(contentType); + } + for (String name : map.keySet()) { + for (Object value : map.get(name)) { + if (value != null && !(value instanceof String)) { + return true; + } + } + } + return false; + } + + private Mono writeMultipart(MultiValueMap map, ReactiveHttpOutputMessage outputMessage) { byte[] boundary = generateMultipartBoundary(); Map params = new HashMap<>(2); params.put("boundary", new String(boundary, StandardCharsets.US_ASCII)); params.put("charset", getCharset().name()); + outputMessage.getHeaders().setContentType(new MediaType(MediaType.MULTIPART_FORM_DATA, params)); - return Mono.from(inputStream).flatMap(map -> { - Flux body = Flux.fromIterable(map.entrySet()) - .concatMap(entry -> encodePartValues(boundary, entry.getKey(), entry.getValue())) - .concatWith(Mono.just(generateLastLine(boundary))); - return outputMessage.writeWith(body); - }); + Flux body = Flux.fromIterable(map.entrySet()) + .concatMap(entry -> encodePartValues(boundary, entry.getKey(), entry.getValue())) + .concatWith(Mono.just(generateLastLine(boundary))); + + return outputMessage.writeWith(body); } /** diff --git a/spring-web/src/main/java/org/springframework/http/codec/support/DefaultClientCodecConfigurer.java b/spring-web/src/main/java/org/springframework/http/codec/support/DefaultClientCodecConfigurer.java index 6abe9a2a2b..15a265bb3b 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/support/DefaultClientCodecConfigurer.java +++ b/spring-web/src/main/java/org/springframework/http/codec/support/DefaultClientCodecConfigurer.java @@ -99,12 +99,11 @@ public class DefaultClientCodecConfigurer extends AbstractCodecConfigurer implem return Collections.emptyList(); } List> result = super.getTypedWriters(); - result.add(new FormHttpMessageWriter()); - result.add(getMultipartHttpMessageWriter()); + result.add(new MultipartHttpMessageWriter(getPartWriters(), new FormHttpMessageWriter())); return result; } - private MultipartHttpMessageWriter getMultipartHttpMessageWriter() { + private List> getPartWriters() { List> partWriters; if (this.multipartCodecs != null) { partWriters = this.multipartCodecs.getWriters(); @@ -122,7 +121,7 @@ public class DefaultClientCodecConfigurer extends AbstractCodecConfigurer implem } partWriters.addAll(super.getCatchAllWriters()); } - return new MultipartHttpMessageWriter(partWriters); + return partWriters; } } diff --git a/spring-web/src/test/java/org/springframework/http/codec/FormHttpMessageWriterTests.java b/spring-web/src/test/java/org/springframework/http/codec/FormHttpMessageWriterTests.java index 38b77a5aec..0b024adc9d 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/FormHttpMessageWriterTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/FormHttpMessageWriterTests.java @@ -27,7 +27,9 @@ import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; /** * @author Sebastien Deleuze @@ -43,17 +45,18 @@ public class FormHttpMessageWriterTests { ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class), MediaType.APPLICATION_FORM_URLENCODED)); + // No generic information assertTrue(this.writer.canWrite( ResolvableType.forInstance(new LinkedMultiValueMap()), MediaType.APPLICATION_FORM_URLENCODED)); assertFalse(this.writer.canWrite( ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, Object.class), - MediaType.APPLICATION_FORM_URLENCODED)); + null)); assertFalse(this.writer.canWrite( ResolvableType.forClassWithGenerics(MultiValueMap.class, Object.class, String.class), - MediaType.APPLICATION_FORM_URLENCODED)); + null)); assertFalse(this.writer.canWrite( ResolvableType.forClassWithGenerics(Map.class, String.class, String.class), diff --git a/spring-web/src/test/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriterTests.java b/spring-web/src/test/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriterTests.java index 54845da5d0..5a8bed8363 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriterTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriterTests.java @@ -70,7 +70,7 @@ public class MultipartHttpMessageWriterTests { assertFalse(this.writer.canWrite( ResolvableType.forClassWithGenerics(Map.class, String.class, Object.class), MediaType.MULTIPART_FORM_DATA)); - assertFalse(this.writer.canWrite( + assertTrue(this.writer.canWrite( ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, Object.class), MediaType.APPLICATION_FORM_URLENCODED)); } diff --git a/spring-web/src/test/java/org/springframework/http/codec/support/ClientCodecConfigurerTests.java b/spring-web/src/test/java/org/springframework/http/codec/support/ClientCodecConfigurerTests.java index 98393d309c..ff59c9b79d 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/support/ClientCodecConfigurerTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/support/ClientCodecConfigurerTests.java @@ -42,7 +42,6 @@ import org.springframework.http.MediaType; import org.springframework.http.codec.ClientCodecConfigurer; import org.springframework.http.codec.DecoderHttpMessageReader; import org.springframework.http.codec.EncoderHttpMessageWriter; -import org.springframework.http.codec.FormHttpMessageWriter; import org.springframework.http.codec.HttpMessageReader; import org.springframework.http.codec.HttpMessageWriter; import org.springframework.http.codec.ResourceHttpMessageWriter; @@ -56,8 +55,11 @@ import org.springframework.http.codec.xml.Jaxb2XmlDecoder; import org.springframework.http.codec.xml.Jaxb2XmlEncoder; import org.springframework.util.MimeTypeUtils; -import static org.junit.Assert.*; -import static org.springframework.core.ResolvableType.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.springframework.core.ResolvableType.forClass; /** * Unit tests for {@link ClientCodecConfigurer}. @@ -90,13 +92,12 @@ public class ClientCodecConfigurerTests { @Test public void defaultWriters() { List> writers = this.configurer.getWriters(); - assertEquals(11, writers.size()); + assertEquals(10, writers.size()); assertEquals(ByteArrayEncoder.class, getNextEncoder(writers).getClass()); assertEquals(ByteBufferEncoder.class, getNextEncoder(writers).getClass()); assertEquals(DataBufferEncoder.class, getNextEncoder(writers).getClass()); assertEquals(ResourceHttpMessageWriter.class, writers.get(index.getAndIncrement()).getClass()); assertStringEncoder(getNextEncoder(writers), true); - assertEquals(FormHttpMessageWriter.class, writers.get(this.index.getAndIncrement()).getClass()); assertEquals(MultipartHttpMessageWriter.class, writers.get(this.index.getAndIncrement()).getClass()); assertEquals(Jackson2JsonEncoder.class, getNextEncoder(writers).getClass()); assertEquals(Jackson2SmileEncoder.class, getNextEncoder(writers).getClass()); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/MultipartIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/MultipartIntegrationTests.java index ffefb84036..1ed3236276 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/MultipartIntegrationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/MultipartIntegrationTests.java @@ -52,8 +52,7 @@ public class MultipartIntegrationTests extends AbstractRouterFunctionIntegration Mono result = webClient .post() .uri("http://localhost:" + this.port + "/multipartData") - .contentType(MediaType.MULTIPART_FORM_DATA) - .body(BodyInserters.fromMultipartData(generateBody())) + .syncBody(generateBody()) .exchange(); StepVerifier @@ -67,8 +66,7 @@ public class MultipartIntegrationTests extends AbstractRouterFunctionIntegration Mono result = webClient .post() .uri("http://localhost:" + this.port + "/parts") - .contentType(MediaType.MULTIPART_FORM_DATA) - .body(BodyInserters.fromMultipartData(generateBody())) + .syncBody(generateBody()) .exchange(); StepVerifier diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MultipartIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MultipartIntegrationTests.java index f3dae0600d..7f3155050c 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MultipartIntegrationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MultipartIntegrationTests.java @@ -50,7 +50,6 @@ import org.springframework.web.bind.annotation.RequestPart; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.reactive.DispatcherHandler; import org.springframework.web.reactive.config.EnableWebFlux; -import org.springframework.web.reactive.function.BodyInserters; import org.springframework.web.reactive.function.client.ClientResponse; import org.springframework.web.reactive.function.client.WebClient; import org.springframework.web.server.adapter.WebHttpHandlerBuilder; @@ -83,8 +82,7 @@ public class MultipartIntegrationTests extends AbstractHttpHandlerIntegrationTes Mono result = webClient .post() .uri("/requestPart") - .contentType(MediaType.MULTIPART_FORM_DATA) - .body(BodyInserters.fromMultipartData(generateBody())) + .syncBody(generateBody()) .exchange(); StepVerifier @@ -98,8 +96,7 @@ public class MultipartIntegrationTests extends AbstractHttpHandlerIntegrationTes Mono result = webClient .post() .uri("/requestBodyMap") - .contentType(MediaType.MULTIPART_FORM_DATA) - .body(BodyInserters.fromMultipartData(generateBody())) + .syncBody(generateBody()) .retrieve() .bodyToMono(String.class); @@ -114,8 +111,7 @@ public class MultipartIntegrationTests extends AbstractHttpHandlerIntegrationTes Mono result = webClient .post() .uri("/requestBodyFlux") - .contentType(MediaType.MULTIPART_FORM_DATA) - .body(BodyInserters.fromMultipartData(generateBody())) + .syncBody(generateBody()) .retrieve() .bodyToMono(String.class); @@ -130,8 +126,7 @@ public class MultipartIntegrationTests extends AbstractHttpHandlerIntegrationTes Mono result = webClient .post() .uri("/modelAttribute") - .contentType(MediaType.MULTIPART_FORM_DATA) - .body(BodyInserters.fromMultipartData(generateBody())) + .syncBody(generateBody()) .retrieve() .bodyToMono(String.class);