From 94cefe6d633ceffa0b2cfc229d97b6d2a26a6f16 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Thu, 4 Jun 2020 17:32:06 +0200 Subject: [PATCH] Respect MediaType charset in Jackson converters Before this commit, AbstractJackson2HttpMessageConverter and subclasses did not check media type encoding in the canRead and canWrite methods. As a result, the converter reported that it can write (for instance) "application/json;charset=ISO-8859-1", but in practice wrote the default charset (UTF-8). This commit fixes that bug. See: gh-25076 --- .../AbstractJackson2HttpMessageConverter.java | 42 +++++++++++++++++-- ...pingJackson2HttpMessageConverterTests.java | 4 ++ ...ackson2SmileHttpMessageConverterTests.java | 5 +++ ...gJackson2XmlHttpMessageConverterTests.java | 4 ++ 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java index 3c49fd268eab..9ad1a320105f 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java @@ -21,7 +21,11 @@ import java.nio.charset.Charset; import java.util.Arrays; import java.util.Collections; +import java.util.EnumSet; +import java.util.Map; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; +import java.util.stream.Collectors; import com.fasterxml.jackson.core.JsonEncoding; import com.fasterxml.jackson.core.JsonGenerator; @@ -68,6 +72,8 @@ */ public abstract class AbstractJackson2HttpMessageConverter extends AbstractGenericHttpMessageConverter { + private static final Map ENCODINGS = jsonEncodings(); + /** * The default charset used by the converter. */ @@ -167,6 +173,14 @@ public boolean canRead(Type type, @Nullable Class contextClass, @Nullable Med return false; } + @Override + protected boolean canRead(@Nullable MediaType mediaType) { + if (!super.canRead(mediaType)) { + return false; + } + return checkEncoding(mediaType); + } + @Override public boolean canWrite(Class clazz, @Nullable MediaType mediaType) { if (!canWrite(mediaType)) { @@ -180,6 +194,14 @@ public boolean canWrite(Class clazz, @Nullable MediaType mediaType) { return false; } + @Override + protected boolean canWrite(@Nullable MediaType mediaType) { + if (!super.canWrite(mediaType)) { + return false; + } + return checkEncoding(mediaType); + } + /** * Determine whether to log the given exception coming from a * {@link ObjectMapper#canDeserialize} / {@link ObjectMapper#canSerialize} check. @@ -211,6 +233,14 @@ else if (logger.isDebugEnabled()) { } } + private boolean checkEncoding(@Nullable MediaType mediaType) { + if (mediaType != null && mediaType.getCharset() != null) { + Charset charset = mediaType.getCharset(); + return ENCODINGS.containsKey(charset.name()); + } + return true; + } + @Override protected Object readInternal(Class clazz, HttpInputMessage inputMessage) throws IOException, HttpMessageNotReadableException { @@ -333,10 +363,9 @@ protected JavaType getJavaType(Type type, @Nullable Class contextClass) { protected JsonEncoding getJsonEncoding(@Nullable MediaType contentType) { if (contentType != null && contentType.getCharset() != null) { Charset charset = contentType.getCharset(); - for (JsonEncoding encoding : JsonEncoding.values()) { - if (charset.name().equals(encoding.getJavaName())) { - return encoding; - } + JsonEncoding encoding = ENCODINGS.get(charset.name()); + if (encoding != null) { + return encoding; } } return JsonEncoding.UTF8; @@ -359,4 +388,9 @@ protected Long getContentLength(Object object, @Nullable MediaType contentType) return super.getContentLength(object, contentType); } + private static Map jsonEncodings() { + return EnumSet.allOf(JsonEncoding.class).stream() + .collect(Collectors.toMap(JsonEncoding::getJavaName, Function.identity())); + } + } diff --git a/spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java index a630f6cd781e..c3f25bff6d54 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java @@ -64,12 +64,16 @@ public class MappingJackson2HttpMessageConverterTests { public void canRead() { assertThat(converter.canRead(MyBean.class, new MediaType("application", "json"))).isTrue(); assertThat(converter.canRead(Map.class, new MediaType("application", "json"))).isTrue(); + assertThat(converter.canRead(MyBean.class, new MediaType("application", "json", StandardCharsets.UTF_8))).isTrue(); + assertThat(converter.canRead(MyBean.class, new MediaType("application", "json", StandardCharsets.ISO_8859_1))).isFalse(); } @Test public void canWrite() { assertThat(converter.canWrite(MyBean.class, new MediaType("application", "json"))).isTrue(); assertThat(converter.canWrite(Map.class, new MediaType("application", "json"))).isTrue(); + assertThat(converter.canWrite(MyBean.class, new MediaType("application", "json", StandardCharsets.UTF_8))).isTrue(); + assertThat(converter.canWrite(MyBean.class, new MediaType("application", "json", StandardCharsets.ISO_8859_1))).isFalse(); } @Test // SPR-7905 diff --git a/spring-web/src/test/java/org/springframework/http/converter/smile/MappingJackson2SmileHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/smile/MappingJackson2SmileHttpMessageConverterTests.java index 2cbb22bdef35..3d6c8912f5c5 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/smile/MappingJackson2SmileHttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/smile/MappingJackson2SmileHttpMessageConverterTests.java @@ -17,6 +17,7 @@ package org.springframework.http.converter.smile; import java.io.IOException; +import java.nio.charset.StandardCharsets; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.smile.SmileFactory; @@ -45,6 +46,8 @@ public void canRead() { assertThat(converter.canRead(MyBean.class, new MediaType("application", "x-jackson-smile"))).isTrue(); assertThat(converter.canRead(MyBean.class, new MediaType("application", "json"))).isFalse(); assertThat(converter.canRead(MyBean.class, new MediaType("application", "xml"))).isFalse(); + assertThat(converter.canRead(MyBean.class, new MediaType("application", "x-jackson-smile", StandardCharsets.UTF_8))).isTrue(); + assertThat(converter.canRead(MyBean.class, new MediaType("application", "x-jackson-smile", StandardCharsets.ISO_8859_1))).isFalse(); } @Test @@ -52,6 +55,8 @@ public void canWrite() { assertThat(converter.canWrite(MyBean.class, new MediaType("application", "x-jackson-smile"))).isTrue(); assertThat(converter.canWrite(MyBean.class, new MediaType("application", "json"))).isFalse(); assertThat(converter.canWrite(MyBean.class, new MediaType("application", "xml"))).isFalse(); + assertThat(converter.canWrite(MyBean.class, new MediaType("application", "x-jackson-smile", StandardCharsets.UTF_8))).isTrue(); + assertThat(converter.canWrite(MyBean.class, new MediaType("application", "x-jackson-smile", StandardCharsets.ISO_8859_1))).isFalse(); } @Test diff --git a/spring-web/src/test/java/org/springframework/http/converter/xml/MappingJackson2XmlHttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/xml/MappingJackson2XmlHttpMessageConverterTests.java index 154129515793..f39747b26ffd 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/xml/MappingJackson2XmlHttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/xml/MappingJackson2XmlHttpMessageConverterTests.java @@ -50,6 +50,8 @@ public void canRead() { assertThat(converter.canRead(MyBean.class, new MediaType("application", "xml"))).isTrue(); assertThat(converter.canRead(MyBean.class, new MediaType("text", "xml"))).isTrue(); assertThat(converter.canRead(MyBean.class, new MediaType("application", "soap+xml"))).isTrue(); + assertThat(converter.canRead(MyBean.class, new MediaType("text", "xml", StandardCharsets.UTF_8))).isTrue(); + assertThat(converter.canRead(MyBean.class, new MediaType("text", "xml", StandardCharsets.ISO_8859_1))).isFalse(); } @Test @@ -57,6 +59,8 @@ public void canWrite() { assertThat(converter.canWrite(MyBean.class, new MediaType("application", "xml"))).isTrue(); assertThat(converter.canWrite(MyBean.class, new MediaType("text", "xml"))).isTrue(); assertThat(converter.canWrite(MyBean.class, new MediaType("application", "soap+xml"))).isTrue(); + assertThat(converter.canWrite(MyBean.class, new MediaType("text", "xml", StandardCharsets.UTF_8))).isTrue(); + assertThat(converter.canWrite(MyBean.class, new MediaType("text", "xml", StandardCharsets.ISO_8859_1))).isFalse(); } @Test