From b8715811f816d5dd713d7ec508b8a6943a7f8c03 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Thu, 7 Dec 2023 12:12:04 +0000 Subject: [PATCH] Write form data without charset parameter This commit updates the content-type for form data to not include the charset, unless it is different from the default charset. Some servers don't handle charset parameter and the spec is unclear. See gh-31781 --- .../http/codec/FormHttpMessageWriter.java | 29 +++++++------- .../converter/FormHttpMessageConverter.java | 38 +++++++------------ .../codec/FormHttpMessageWriterTests.java | 2 +- .../FormHttpMessageConverterTests.java | 17 +++++---- .../client/AbstractMockWebServerTests.java | 4 +- .../support/RestClientAdapterTests.java | 2 +- ...KotlinRestTemplateHttpServiceProxyTests.kt | 3 +- .../client/support/WebClientAdapterTests.java | 2 +- 8 files changed, 44 insertions(+), 53 deletions(-) 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 f4867dcb82e..7b80cbb7f8d 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 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. @@ -60,14 +60,9 @@ import org.springframework.util.MultiValueMap; public class FormHttpMessageWriter extends LoggingCodecSupport implements HttpMessageWriter> { - /** - * The default charset used by the writer. - */ + /** The default charset used by the writer. */ public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8; - private static final MediaType DEFAULT_FORM_DATA_MEDIA_TYPE = - new MediaType(MediaType.APPLICATION_FORM_URLENCODED, DEFAULT_CHARSET); - private static final List MEDIA_TYPES = Collections.singletonList(MediaType.APPLICATION_FORM_URLENCODED); @@ -126,7 +121,7 @@ public class FormHttpMessageWriter extends LoggingCodecSupport mediaType = getMediaType(mediaType); message.getHeaders().setContentType(mediaType); - Charset charset = mediaType.getCharset() != null ? mediaType.getCharset() : getDefaultCharset(); + Charset charset = (mediaType.getCharset() != null ? mediaType.getCharset() : getDefaultCharset()); return Mono.from(inputStream).flatMap(form -> { logFormData(form, hints); @@ -138,16 +133,22 @@ public class FormHttpMessageWriter extends LoggingCodecSupport }); } + /** + * Return the content type used to write forms, either the given media type + * or otherwise {@code application/x-www-form-urlencoded}. + * @param mediaType the media type passed to {@link #write}, or {@code null} + * @return the content type to use + */ protected MediaType getMediaType(@Nullable MediaType mediaType) { if (mediaType == null) { - return DEFAULT_FORM_DATA_MEDIA_TYPE; - } - else if (mediaType.getCharset() == null) { - return new MediaType(mediaType, getDefaultCharset()); + return MediaType.APPLICATION_FORM_URLENCODED; } - else { - return mediaType; + // Some servers don't handle charset parameter and spec is unclear, + // Add it only if it is not DEFAULT_CHARSET. + if (mediaType.getCharset() == null && this.defaultCharset != DEFAULT_CHARSET) { + return new MediaType(mediaType, this.defaultCharset); } + return mediaType; } private void logFormData(MultiValueMap form, Map hints) { 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 d7a64f2cf5d..ed95e0c2acd 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 @@ -154,14 +154,9 @@ import org.springframework.util.StringUtils; */ public class FormHttpMessageConverter implements HttpMessageConverter> { - /** - * The default charset used by the converter. - */ + /** The default charset used by the converter. */ public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8; - private static final MediaType DEFAULT_FORM_DATA_MEDIA_TYPE = - new MediaType(MediaType.APPLICATION_FORM_URLENCODED, DEFAULT_CHARSET); - private List supportedMediaTypes = new ArrayList<>(); @@ -387,14 +382,13 @@ public class FormHttpMessageConverter implements HttpMessageConverter formData, @Nullable MediaType contentType, + private void writeForm(MultiValueMap formData, @Nullable MediaType mediaType, HttpOutputMessage outputMessage) throws IOException { - contentType = getFormContentType(contentType); - outputMessage.getHeaders().setContentType(contentType); + mediaType = getFormContentType(mediaType); + outputMessage.getHeaders().setContentType(mediaType); - Charset charset = contentType.getCharset(); - Assert.notNull(charset, "No charset"); // should never occur + Charset charset = (mediaType.getCharset() != null ? mediaType.getCharset() : this.charset); byte[] bytes = serializeForm(formData, charset).getBytes(charset); outputMessage.getHeaders().setContentLength(bytes.length); @@ -418,26 +412,22 @@ public class FormHttpMessageConverter implements HttpMessageConverterSubclasses can override this method to change this behavior. - * @param contentType the preferred content type (can be {@code null}) - * @return the content type to be used + * Return the content type used to write forms, either the given content type + * or otherwise {@code application/x-www-form-urlencoded}. + * @param contentType the content type passed to {@link #write}, or {@code null} + * @return the content type to use * @since 5.2.2 */ protected MediaType getFormContentType(@Nullable MediaType contentType) { if (contentType == null) { - return DEFAULT_FORM_DATA_MEDIA_TYPE; + return MediaType.APPLICATION_FORM_URLENCODED; } - else if (contentType.getCharset() == null) { + // Some servers don't handle charset parameter and spec is unclear, + // Add it only if it is not DEFAULT_CHARSET. + if (contentType.getCharset() == null && this.charset != DEFAULT_CHARSET) { return new MediaType(contentType, this.charset); } - else { - return contentType; - } + return contentType; } protected String serializeForm(MultiValueMap formData, Charset charset) { 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 e51b297db2f..d0de1dcaf8b 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 @@ -88,7 +88,7 @@ class FormHttpMessageWriterTests extends AbstractLeakCheckingTests { .expectComplete() .verify(); HttpHeaders headers = response.getHeaders(); - assertThat(headers.getContentType().toString()).isEqualTo("application/x-www-form-urlencoded;charset=UTF-8"); + assertThat(headers.getContentType()).isEqualTo(MediaType.APPLICATION_FORM_URLENCODED); assertThat(headers.getContentLength()).isEqualTo(expected.length()); } 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 cf1c9c1a8da..98c937bc733 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 @@ -47,6 +47,7 @@ import org.springframework.util.MultiValueMap; import org.springframework.web.testfixture.http.MockHttpInputMessage; import org.springframework.web.testfixture.http.MockHttpOutputMessage; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED; import static org.springframework.http.MediaType.APPLICATION_JSON; @@ -93,7 +94,7 @@ class FormHttpMessageConverterTests { assertCanWrite(MULTIPART_FORM_DATA); assertCanWrite(MULTIPART_MIXED); assertCanWrite(MULTIPART_RELATED); - assertCanWrite(new MediaType("multipart", "form-data", StandardCharsets.UTF_8)); + assertCanWrite(new MediaType("multipart", "form-data", UTF_8)); assertCanWrite(MediaType.ALL); assertCanWrite(null); } @@ -141,10 +142,10 @@ class FormHttpMessageConverterTests { MockHttpOutputMessage outputMessage = new MockHttpOutputMessage(); this.converter.write(body, APPLICATION_FORM_URLENCODED, outputMessage); - assertThat(outputMessage.getBodyAsString(StandardCharsets.UTF_8)) + assertThat(outputMessage.getBodyAsString(UTF_8)) .as("Invalid result").isEqualTo("name+1=value+1&name+2=value+2%2B1&name+2=value+2%2B2&name+3"); - assertThat(outputMessage.getHeaders().getContentType().toString()) - .as("Invalid content-type").isEqualTo("application/x-www-form-urlencoded;charset=UTF-8"); + assertThat(outputMessage.getHeaders().getContentType()) + .as("Invalid content-type").isEqualTo(APPLICATION_FORM_URLENCODED); assertThat(outputMessage.getHeaders().getContentLength()) .as("Invalid content-length").isEqualTo(outputMessage.getBodyAsBytes().length); } @@ -178,7 +179,7 @@ class FormHttpMessageConverterTests { parts.add("json", entity); Map parameters = new LinkedHashMap<>(2); - parameters.put("charset", StandardCharsets.UTF_8.name()); + parameters.put("charset", UTF_8.name()); parameters.put("foo", "bar"); MockHttpOutputMessage outputMessage = new MockHttpOutputMessage(); @@ -260,7 +261,7 @@ class FormHttpMessageConverterTests { parts.add("xml", entity); Map parameters = new LinkedHashMap<>(2); - parameters.put("charset", StandardCharsets.UTF_8.name()); + parameters.put("charset", UTF_8.name()); parameters.put("foo", "bar"); MockHttpOutputMessage outputMessage = new MockHttpOutputMessage(); @@ -323,8 +324,8 @@ class FormHttpMessageConverterTests { parts.add("part2", entity); MockHttpOutputMessage outputMessage = new MockHttpOutputMessage(); - this.converter.setMultipartCharset(StandardCharsets.UTF_8); - this.converter.write(parts, new MediaType("multipart", "form-data", StandardCharsets.UTF_8), outputMessage); + this.converter.setMultipartCharset(UTF_8); + this.converter.write(parts, new MediaType("multipart", "form-data", UTF_8), outputMessage); final MediaType contentType = outputMessage.getHeaders().getContentType(); assertThat(contentType.getParameter("boundary")).as("No boundary found").isNotNull(); diff --git a/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTests.java b/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTests.java index a92c05d2723..9f0efe928ca 100644 --- a/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTests.java +++ b/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTests.java @@ -192,7 +192,7 @@ abstract class AbstractMockWebServerTests { } private MockResponse formRequest(RecordedRequest request) { - assertThat(request.getHeader(CONTENT_TYPE)).isEqualTo("application/x-www-form-urlencoded;charset=UTF-8"); + assertThat(request.getHeader(CONTENT_TYPE)).isEqualTo("application/x-www-form-urlencoded"); assertThat(request.getBody().readUtf8()).contains("name+1=value+1", "name+2=value+2%2B1", "name+2=value+2%2B2"); return new MockResponse().setResponseCode(200); } @@ -235,7 +235,7 @@ abstract class AbstractMockWebServerTests { protected class TestDispatcher extends Dispatcher { @Override - public MockResponse dispatch(RecordedRequest request) throws InterruptedException { + public MockResponse dispatch(RecordedRequest request) { try { byte[] helloWorldBytes = helloWorld.getBytes(StandardCharsets.UTF_8); diff --git a/spring-web/src/test/java/org/springframework/web/client/support/RestClientAdapterTests.java b/spring-web/src/test/java/org/springframework/web/client/support/RestClientAdapterTests.java index 78f8b331092..caa2162d003 100644 --- a/spring-web/src/test/java/org/springframework/web/client/support/RestClientAdapterTests.java +++ b/spring-web/src/test/java/org/springframework/web/client/support/RestClientAdapterTests.java @@ -188,7 +188,7 @@ class RestClientAdapterTests { service.postForm(map); RecordedRequest request = server.takeRequest(); - assertThat(request.getHeaders().get("Content-Type")).isEqualTo("application/x-www-form-urlencoded;charset=UTF-8"); + assertThat(request.getHeaders().get("Content-Type")).isEqualTo("application/x-www-form-urlencoded"); assertThat(request.getBody().readUtf8()).isEqualTo("param1=value+1¶m2=value+2"); } diff --git a/spring-web/src/test/kotlin/org/springframework/web/client/support/KotlinRestTemplateHttpServiceProxyTests.kt b/spring-web/src/test/kotlin/org/springframework/web/client/support/KotlinRestTemplateHttpServiceProxyTests.kt index 4248b27ae33..9ce47123f5b 100644 --- a/spring-web/src/test/kotlin/org/springframework/web/client/support/KotlinRestTemplateHttpServiceProxyTests.kt +++ b/spring-web/src/test/kotlin/org/springframework/web/client/support/KotlinRestTemplateHttpServiceProxyTests.kt @@ -136,8 +136,7 @@ class KotlinRestTemplateHttpServiceProxyTests { testService.postForm(map) val request = server.takeRequest() - assertThat(request.headers["Content-Type"]) - .isEqualTo("application/x-www-form-urlencoded;charset=UTF-8") + assertThat(request.headers["Content-Type"]).isEqualTo("application/x-www-form-urlencoded") assertThat(request.body.readUtf8()).isEqualTo("param1=value+1¶m2=value+2") } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/support/WebClientAdapterTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/support/WebClientAdapterTests.java index 13144f11ccb..5ee065af5ab 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/support/WebClientAdapterTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/support/WebClientAdapterTests.java @@ -145,7 +145,7 @@ class WebClientAdapterTests { initService().postForm(map); RecordedRequest request = this.server.takeRequest(); - assertThat(request.getHeaders().get("Content-Type")).isEqualTo("application/x-www-form-urlencoded;charset=UTF-8"); + assertThat(request.getHeaders().get("Content-Type")).isEqualTo("application/x-www-form-urlencoded"); assertThat(request.getBody().readUtf8()).isEqualTo("param1=value+1¶m2=value+2"); }