Polish form reader/writer

spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageReader.java

@@ -39,29 +39,52 @@ import org.springframework.util.MultiValueMap;
 import org.springframework.util.StringUtils;
 
 /**
- * Implementation of {@link HttpMessageReader} to read 'normal' HTML
- * forms with {@code "application/x-www-form-urlencoded"} media type.
+ * Implementation of an {@link HttpMessageReader} to read HTML form data, i.e.
+ * request body with media type {@code "application/x-www-form-urlencoded"}.
  *
  * @author Sebastien Deleuze
+ * @author Rossen Stoyanchev
+ * @since 5.0
  */
 public class FormHttpMessageReader implements HttpMessageReader<MultiValueMap<String, String>> {
 
 	public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8;
 
-	private static final ResolvableType formType = ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class);
+	private static final ResolvableType MULTIVALUE_TYPE =
+			ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class);
 
-	private Charset charset = DEFAULT_CHARSET;
+
+	private Charset defaultCharset = DEFAULT_CHARSET;
+
+
+	/**
+	 * Set the default character set to use for reading form data when the
+	 * request Content-Type header does not explicitly specify it.
+	 * <p>By default this is set to "UTF-8".
+	 */
+	public void setDefaultCharset(Charset charset) {
+		Assert.notNull(charset, "'charset' must not be null");
+		this.defaultCharset = charset;
+	}
+
+	/**
+	 * Return the configured default charset.
+	 */
+	public Charset getDefaultCharset() {
+		return this.defaultCharset;
+	}
 
 
 	@Override
 	public boolean canRead(ResolvableType elementType, MediaType mediaType) {
-		return (mediaType == null || MediaType.APPLICATION_FORM_URLENCODED.isCompatibleWith(mediaType)) &&
-				formType.isAssignableFrom(elementType);
+		return MULTIVALUE_TYPE.isAssignableFrom(elementType) &&
+				(mediaType == null || MediaType.APPLICATION_FORM_URLENCODED.isCompatibleWith(mediaType));
 	}
 
 	@Override
 	public Flux<MultiValueMap<String, String>> read(ResolvableType elementType,
 			ReactiveHttpInputMessage inputMessage, Map<String, Object> hints) {
+
 		return Flux.from(readMono(elementType, inputMessage, hints));
 	}
 
@@ -70,50 +93,52 @@ public class FormHttpMessageReader implements HttpMessageReader<MultiValueMap<St
 			ReactiveHttpInputMessage inputMessage, Map<String, Object> hints) {
 
 		MediaType contentType = inputMessage.getHeaders().getContentType();
-		Charset charset = (contentType.getCharset() != null ? contentType.getCharset() : this.charset);
+		Charset charset = getMediaTypeCharset(contentType);
 
 		return inputMessage.getBody()
 				.reduce(DataBuffer::write)
 				.map(buffer -> {
 					CharBuffer charBuffer = charset.decode(buffer.asByteBuffer());
-					DataBufferUtils.release(buffer);
 					String body = charBuffer.toString();
-					String[] pairs = StringUtils.tokenizeToStringArray(body, "&");
-					MultiValueMap<String, String> result = new LinkedMultiValueMap<>(pairs.length);
-					try {
-						for (String pair : pairs) {
-							int idx = pair.indexOf('=');
-							if (idx == -1) {
-								result.add(URLDecoder.decode(pair, charset.name()), null);
-							}
-							else {
-								String name = URLDecoder.decode(pair.substring(0, idx), charset.name());
-								String value = URLDecoder.decode(pair.substring(idx + 1), charset.name());
-								result.add(name, value);
-							}
-						}
-					}
-					catch (UnsupportedEncodingException ex) {
-						throw new IllegalStateException(ex);
-					}
-
-					return result;
+					DataBufferUtils.release(buffer);
+					return parseFormData(charset, body);
 				});
 	}
 
+	private Charset getMediaTypeCharset(MediaType mediaType) {
+		if (mediaType != null && mediaType.getCharset() != null) {
+			return mediaType.getCharset();
+		}
+		else {
+			return getDefaultCharset();
+		}
+	}
+
+	private MultiValueMap<String, String> parseFormData(Charset charset, String body) {
+		String[] pairs = StringUtils.tokenizeToStringArray(body, "&");
+		MultiValueMap<String, String> result = new LinkedMultiValueMap<>(pairs.length);
+		try {
+			for (String pair : pairs) {
+				int idx = pair.indexOf('=');
+				if (idx == -1) {
+					result.add(URLDecoder.decode(pair, charset.name()), null);
+				}
+				else {
+					String name = URLDecoder.decode(pair.substring(0, idx),  charset.name());
+					String value = URLDecoder.decode(pair.substring(idx + 1), charset.name());
+					result.add(name, value);
+				}
+			}
+		}
+		catch (UnsupportedEncodingException ex) {
+			throw new IllegalStateException(ex);
+		}
+		return result;
+	}
+
 	@Override
 	public List<MediaType> getReadableMediaTypes() {
 		return Collections.singletonList(MediaType.APPLICATION_FORM_URLENCODED);
 	}
 
-	/**
-	 * Set the default character set to use for reading form data when the request
-	 * Content-Type header does not explicitly specify it.
-	 * <p>By default this is set to "UTF-8".
-	 */
-	public void setCharset(Charset charset) {
-		Assert.notNull(charset, "'charset' must not be null");
-		this.charset = charset;
-	}
-
 }

spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java

@@ -38,26 +38,46 @@ import org.springframework.util.Assert;
 import org.springframework.util.MultiValueMap;
 
 /**
- * Implementation of {@link HttpMessageWriter} to write 'normal' HTML
- * forms with {@code "application/x-www-form-urlencoded"} media type.
+ * Implementation of an {@link HttpMessageWriter} to write HTML form data, i.e.
+ * response body with media type {@code "application/x-www-form-urlencoded"}.
  *
  * @author Sebastien Deleuze
+ * @author Rossen Stoyanchev
  * @since 5.0
- * @see MultiValueMap
  */
 public class FormHttpMessageWriter implements HttpMessageWriter<MultiValueMap<String, String>> {
 
 	public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8;
 
-	private static final ResolvableType formType = ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class);
+	private static final ResolvableType MULTIVALUE_TYPE =
+			ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class);
 
-	private Charset charset = DEFAULT_CHARSET;
+
+	private Charset defaultCharset = DEFAULT_CHARSET;
+
+
+	/**
+	 * Set the default character set to use for writing form data when the response
+	 * Content-Type header does not explicitly specify it.
+	 * <p>By default this is set to "UTF-8".
+	 */
+	public void setDefaultCharset(Charset charset) {
+		Assert.notNull(charset, "'charset' must not be null");
+		this.defaultCharset = charset;
+	}
+
+	/**
+	 * Return the configured default charset.
+	 */
+	public Charset getDefaultCharset() {
+		return this.defaultCharset;
+	}
 
 
 	@Override
 	public boolean canWrite(ResolvableType elementType, MediaType mediaType) {
-		return (mediaType == null || MediaType.APPLICATION_FORM_URLENCODED.isCompatibleWith(mediaType)) &&
-				formType.isAssignableFrom(elementType);
+		return MULTIVALUE_TYPE.isAssignableFrom(elementType) &&
+				(mediaType == null || MediaType.APPLICATION_FORM_URLENCODED.isCompatibleWith(mediaType));
 	}
 
 	@Override
@@ -66,19 +86,17 @@ public class FormHttpMessageWriter implements HttpMessageWriter<MultiValueMap<St
 			Map<String, Object> hints) {
 
 		MediaType contentType = outputMessage.getHeaders().getContentType();
-		Charset charset;
-		if (contentType != null) {
+		if (contentType == null) {
+			contentType = MediaType.APPLICATION_FORM_URLENCODED;
 			outputMessage.getHeaders().setContentType(contentType);
-			charset = (contentType != null && contentType.getCharset() != null ? contentType.getCharset() : this.charset);
-		}
-		else {
-			outputMessage.getHeaders().setContentType(MediaType.APPLICATION_FORM_URLENCODED);
-			charset = this.charset;
 		}
+
+		Charset charset = getMediaTypeCharset(contentType);
+
 		return Flux
 				.from(inputStream)
 				.single()
-				.map(form -> generateForm(form))
+				.map(form -> generateForm(form, charset))
 				.then(value -> {
 					ByteBuffer byteBuffer = charset.encode(value);
 					DataBuffer buffer = outputMessage.bufferFactory().wrap(byteBuffer);
@@ -88,23 +106,32 @@ public class FormHttpMessageWriter implements HttpMessageWriter<MultiValueMap<St
 
 	}
 
-	private String generateForm(MultiValueMap<String, String> form) {
+	private Charset getMediaTypeCharset(MediaType mediaType) {
+		if (mediaType != null && mediaType.getCharset() != null) {
+			return mediaType.getCharset();
+		}
+		else {
+			return getDefaultCharset();
+		}
+	}
+
+	private String generateForm(MultiValueMap<String, String> form, Charset charset) {
 		StringBuilder builder = new StringBuilder();
 		try {
-			for (Iterator<String> nameIterator = form.keySet().iterator(); nameIterator.hasNext();) {
-				String name = nameIterator.next();
-				for (Iterator<String> valueIterator = form.get(name).iterator(); valueIterator.hasNext();) {
-					String value = valueIterator.next();
+			for (Iterator<String> names = form.keySet().iterator(); names.hasNext();) {
+				String name = names.next();
+				for (Iterator<String> values = form.get(name).iterator(); values.hasNext();) {
+					String value = values.next();
 					builder.append(URLEncoder.encode(name, charset.name()));
 					if (value != null) {
 						builder.append('=');
 						builder.append(URLEncoder.encode(value, charset.name()));
-						if (valueIterator.hasNext()) {
+						if (values.hasNext()) {
 							builder.append('&');
 						}
 					}
 				}
-				if (nameIterator.hasNext()) {
+				if (names.hasNext()) {
 					builder.append('&');
 				}
 			}
@@ -120,14 +147,4 @@ public class FormHttpMessageWriter implements HttpMessageWriter<MultiValueMap<St
 		return Collections.singletonList(MediaType.APPLICATION_FORM_URLENCODED);
 	}
 
-	/**
-	 * Set the default character set to use for writing form data when the response
-	 * Content-Type header does not explicitly specify it.
-	 * <p>By default this is set to "UTF-8".
-	 */
-	public void setCharset(Charset charset) {
-		Assert.notNull(charset, "'charset' must not be null");
-		this.charset = charset;
-	}
-
 }

spring-web/src/test/java/org/springframework/http/codec/FormHttpMessageReaderTests.java

@@ -36,15 +36,24 @@ public class FormHttpMessageReaderTests {
 
 	@Test
 	public void canRead() {
-		assertTrue(this.reader.canRead(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class),
+		assertTrue(this.reader.canRead(
+				ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class),
 				MediaType.APPLICATION_FORM_URLENCODED));
-		assertFalse(this.reader.canRead(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, Object.class),
+
+		assertFalse(this.reader.canRead(
+				ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, Object.class),
 				MediaType.APPLICATION_FORM_URLENCODED));
-		assertFalse(this.reader.canRead(ResolvableType.forClassWithGenerics(MultiValueMap.class, Object.class, String.class),
+
+		assertFalse(this.reader.canRead(
+				ResolvableType.forClassWithGenerics(MultiValueMap.class, Object.class, String.class),
 				MediaType.APPLICATION_FORM_URLENCODED));
-		assertFalse(this.reader.canRead(ResolvableType.forClassWithGenerics(Map.class, String.class, String.class),
+
+		assertFalse(this.reader.canRead(
+				ResolvableType.forClassWithGenerics(Map.class, String.class, String.class),
 				MediaType.APPLICATION_FORM_URLENCODED));
-		assertFalse(this.reader.canRead(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class),
+
+		assertFalse(this.reader.canRead(
+				ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class),
 				MediaType.MULTIPART_FORM_DATA));
 	}
 

spring-web/src/test/java/org/springframework/http/codec/FormHttpMessageWriterTests.java

@@ -37,17 +37,27 @@ public class FormHttpMessageWriterTests {
 
 	private final FormHttpMessageWriter writer = new FormHttpMessageWriter();
 
+
 	@Test
 	public void canWrite() {
-		assertTrue(this.writer.canWrite(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class),
+		assertTrue(this.writer.canWrite(
+				ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class),
 				MediaType.APPLICATION_FORM_URLENCODED));
-		assertFalse(this.writer.canWrite(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, Object.class),
+
+		assertFalse(this.writer.canWrite(
+				ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, Object.class),
 				MediaType.APPLICATION_FORM_URLENCODED));
-		assertFalse(this.writer.canWrite(ResolvableType.forClassWithGenerics(MultiValueMap.class, Object.class, String.class),
+
+		assertFalse(this.writer.canWrite(
+				ResolvableType.forClassWithGenerics(MultiValueMap.class, Object.class, String.class),
 				MediaType.APPLICATION_FORM_URLENCODED));
-		assertFalse(this.writer.canWrite(ResolvableType.forClassWithGenerics(Map.class, String.class, String.class),
+
+		assertFalse(this.writer.canWrite(
+				ResolvableType.forClassWithGenerics(Map.class, String.class, String.class),
 				MediaType.APPLICATION_FORM_URLENCODED));
-		assertFalse(this.writer.canWrite(ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class),
+
+		assertFalse(this.writer.canWrite(
+				ResolvableType.forClassWithGenerics(MultiValueMap.class, String.class, String.class),
 				MediaType.MULTIPART_FORM_DATA));
 	}
 
@@ -62,12 +72,9 @@ public class FormHttpMessageWriterTests {
 		this.writer.write(Mono.just(body), null, MediaType.APPLICATION_FORM_URLENCODED, response, null).block();
 
 		String responseBody = response.getBodyAsString().block();
-		assertEquals("Invalid result", "name+1=value+1&name+2=value+2%2B1&name+2=value+2%2B2&name+3",
-				responseBody);
-		assertEquals("Invalid content-type", MediaType.APPLICATION_FORM_URLENCODED,
-				response.getHeaders().getContentType());
-		assertEquals("Invalid content-length", responseBody.getBytes().length,
-				response.getHeaders().getContentLength());
+		assertEquals("name+1=value+1&name+2=value+2%2B1&name+2=value+2%2B2&name+3", responseBody);
+		assertEquals(MediaType.APPLICATION_FORM_URLENCODED, response.getHeaders().getContentType());
+		assertEquals(responseBody.getBytes().length, response.getHeaders().getContentLength());
 	}
 
 }