Skip to content

Commit

Permalink
Respect MediaType charset in Jackson converters
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@poutsma
poutsma committed Jun 5, 2020
1 parent e11da64 commit 6a829a3
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 4 deletions.
42 changes: 38 additions & 4 deletions ...in/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -68,6 +72,8 @@
*/
public abstract class AbstractJackson2HttpMessageConverter extends AbstractGenericHttpMessageConverter<Object> {

private static final Map<String, JsonEncoding> ENCODINGS = jsonEncodings();

/**
* The default charset used by the converter.
*/
Expand Down Expand Up @@ -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)) {
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand All @@ -359,4 +388,9 @@ protected Long getContentLength(Object object, @Nullable MediaType contentType)
return super.getContentLength(object, contentType);
}

private static Map<String, JsonEncoding> jsonEncodings() {
return EnumSet.allOf(JsonEncoding.class).stream()
.collect(Collectors.toMap(JsonEncoding::getJavaName, Function.identity()));
}

}
4 changes: 4 additions & 0 deletions ...ava/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java
View file
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions ...g/springframework/http/converter/smile/MappingJackson2SmileHttpMessageConverterTests.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -45,13 +46,17 @@ 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
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
Expand Down
4 changes: 4 additions & 0 deletions ...a/org/springframework/http/converter/xml/MappingJackson2XmlHttpMessageConverterTests.java
View file
Expand Up @@ -50,13 +50,17 @@ 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
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
Expand Down

0 comments on commit 6a829a3

Please sign in to comment.