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: spring-projectsgh-25076
  • Loading branch information
@poutsma @kenny5he
poutsma authored and kenny5he committed Jun 21, 2020
1 parent 8bed581 commit 0daf13e
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 75 deletions.
73 changes: 31 additions & 42 deletions ...in/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java
View file
Expand Up @@ -17,11 +17,8 @@
package org.springframework.http.converter.json;

import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Reader;
import java.lang.reflect.Type;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collections;
import java.util.EnumSet;
Expand All @@ -39,7 +36,6 @@
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectReader;
import com.fasterxml.jackson.databind.ObjectWriter;
import com.fasterxml.jackson.databind.SerializationConfig;
import com.fasterxml.jackson.databind.SerializationFeature;
Expand Down Expand Up @@ -76,7 +72,7 @@
*/
public abstract class AbstractJackson2HttpMessageConverter extends AbstractGenericHttpMessageConverter<Object> {

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

/**
* The default charset used by the converter.
Expand Down Expand Up @@ -177,17 +173,19 @@ 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)) {
return false;
}
if (mediaType != null && mediaType.getCharset() != null) {
Charset charset = mediaType.getCharset();
if (!ENCODINGS.containsKey(charset)) {
return false;
}
}
AtomicReference<Throwable> causeRef = new AtomicReference<>();
if (this.objectMapper.canSerialize(clazz, causeRef)) {
return true;
Expand All @@ -196,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 @@ -227,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 All @@ -244,31 +258,15 @@ public Object read(Type type, @Nullable Class<?> contextClass, HttpInputMessage
}

private Object readJavaType(JavaType javaType, HttpInputMessage inputMessage) throws IOException {
MediaType contentType = inputMessage.getHeaders().getContentType();
Charset charset = getCharset(contentType);

boolean isUnicode = ENCODINGS.containsKey(charset);
try {
if (inputMessage instanceof MappingJacksonInputMessage) {
Class<?> deserializationView = ((MappingJacksonInputMessage) inputMessage).getDeserializationView();
if (deserializationView != null) {
ObjectReader objectReader = this.objectMapper.readerWithView(deserializationView).forType(javaType);
if (isUnicode) {
return objectReader.readValue(inputMessage.getBody());
}
else {
Reader reader = new InputStreamReader(inputMessage.getBody(), charset);
return objectReader.readValue(reader);
}
return this.objectMapper.readerWithView(deserializationView).forType(javaType).
readValue(inputMessage.getBody());
}
}
if (isUnicode) {
return this.objectMapper.readValue(inputMessage.getBody(), javaType);
}
else {
Reader reader = new InputStreamReader(inputMessage.getBody(), charset);
return this.objectMapper.readValue(reader, javaType);
}
return this.objectMapper.readValue(inputMessage.getBody(), javaType);
}
catch (InvalidDefinitionException ex) {
throw new HttpMessageConversionException("Type definition error: " + ex.getType(), ex);
Expand All @@ -278,15 +276,6 @@ private Object readJavaType(JavaType javaType, HttpInputMessage inputMessage) th
}
}

private static Charset getCharset(@Nullable MediaType contentType) {
if (contentType != null && contentType.getCharset() != null) {
return contentType.getCharset();
}
else {
return StandardCharsets.UTF_8;
}
}

@Override
protected void writeInternal(Object object, @Nullable Type type, HttpOutputMessage outputMessage)
throws IOException, HttpMessageNotWritableException {
Expand Down Expand Up @@ -374,7 +363,7 @@ protected JavaType getJavaType(Type type, @Nullable Class<?> contextClass) {
protected JsonEncoding getJsonEncoding(@Nullable MediaType contentType) {
if (contentType != null && contentType.getCharset() != null) {
Charset charset = contentType.getCharset();
JsonEncoding encoding = ENCODINGS.get(charset);
JsonEncoding encoding = ENCODINGS.get(charset.name());
if (encoding != null) {
return encoding;
}
Expand All @@ -399,9 +388,9 @@ protected Long getContentLength(Object object, @Nullable MediaType contentType)
return super.getContentLength(object, contentType);
}

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

}
18 changes: 2 additions & 16 deletions ...ava/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java
View file
Expand Up @@ -18,7 +18,6 @@

import java.io.IOException;
import java.lang.reflect.Type;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.HashMap;
Expand All @@ -45,7 +44,6 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.entry;
import static org.assertj.core.api.Assertions.within;

/**
Expand All @@ -67,7 +65,7 @@ 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))).isTrue();
assertThat(converter.canRead(MyBean.class, new MediaType("application", "json", StandardCharsets.ISO_8859_1))).isFalse();
}

@Test
Expand Down Expand Up @@ -441,25 +439,13 @@ public void writeSubTypeList() throws Exception {
@Test
public void readWithNoDefaultConstructor() throws Exception {
String body = "{\"property1\":\"foo\",\"property2\":\"bar\"}";
MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes(StandardCharsets.UTF_8));
MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8"));
inputMessage.getHeaders().setContentType(new MediaType("application", "json"));
assertThatExceptionOfType(HttpMessageConversionException.class).isThrownBy(() ->
converter.read(BeanWithNoDefaultConstructor.class, inputMessage))
.withMessageStartingWith("Type definition error:");
}

@Test
@SuppressWarnings("unchecked")
public void readNonUnicode() throws Exception {
String body = "{\"føø\":\"bår\"}";
Charset charset = StandardCharsets.ISO_8859_1;
MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes(charset));
inputMessage.getHeaders().setContentType(new MediaType("application", "json", charset));
HashMap<String, Object> result = (HashMap<String, Object>) this.converter.read(HashMap.class, inputMessage);

assertThat(result).containsExactly(entry("føø", "bår"));
}


interface MyInterface {

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
18 changes: 1 addition & 17 deletions ...a/org/springframework/http/converter/xml/MappingJackson2XmlHttpMessageConverterTests.java
View file
Expand Up @@ -17,7 +17,6 @@
package org.springframework.http.converter.xml;

import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;

import com.fasterxml.jackson.annotation.JsonView;
Expand Down Expand Up @@ -52,7 +51,7 @@ public void canRead() {
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))).isTrue();
assertThat(converter.canRead(MyBean.class, new MediaType("text", "xml", StandardCharsets.ISO_8859_1))).isFalse();
}

@Test
Expand Down Expand Up @@ -190,21 +189,6 @@ public void readWithXmlBomb() throws IOException {
this.converter.read(MyBean.class, inputMessage));
}

@Test
@SuppressWarnings("unchecked")
public void readNonUnicode() throws Exception {
String body = "<MyBean>" +
"<string>føø bår</string>" +
"</MyBean>";

Charset charset = StandardCharsets.ISO_8859_1;
MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes(charset));
inputMessage.getHeaders().setContentType(new MediaType("application", "xml", charset));
MyBean result = (MyBean) converter.read(MyBean.class, inputMessage);
assertThat(result.getString()).isEqualTo("føø bår");
}



public static class MyBean {

Expand Down

0 comments on commit 0daf13e

Please sign in to comment.