From 38b9d0fb0d8f84cd2316428cc8dc32f1be8d2d83 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Tue, 16 Jun 2020 14:29:02 +0200 Subject: [PATCH] Convert non-unicode input when reading w/ Jackson This commit makes sure that Jackson-based message converters and decoders can deal with non-unicode input. It does so by reading non-unicode input messages with a InputStreamReader. This commit also adds additional tests forthe canRead/canWrite methods on both codecs and message converters. Closes: gh-25247 --- .../codec/json/AbstractJackson2Encoder.java | 26 +++++-- .../http/codec/json/Jackson2CodecSupport.java | 26 +------ .../AbstractJackson2HttpMessageConverter.java | 72 +++++++++++-------- .../codec/json/Jackson2JsonDecoderTests.java | 22 +++++- .../codec/json/Jackson2SmileDecoderTests.java | 6 -- .../codec/json/Jackson2SmileEncoderTests.java | 6 -- ...pingJackson2HttpMessageConverterTests.java | 18 ++++- ...ackson2SmileHttpMessageConverterTests.java | 4 -- ...gJackson2XmlHttpMessageConverterTests.java | 18 ++++- 9 files changed, 117 insertions(+), 81 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Encoder.java b/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Encoder.java index 309a79c1a266..29f8fc789258 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Encoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Encoder.java @@ -69,10 +69,18 @@ public abstract class AbstractJackson2Encoder extends Jackson2CodecSupport imple private static final Map STREAM_SEPARATORS; + private static final Map ENCODINGS; + static { STREAM_SEPARATORS = new HashMap<>(4); STREAM_SEPARATORS.put(MediaType.APPLICATION_STREAM_JSON, NEWLINE_SEPARATOR); STREAM_SEPARATORS.put(MediaType.parseMediaType("application/stream+x-jackson-smile"), new byte[0]); + + ENCODINGS = new HashMap<>(JsonEncoding.values().length); + for (JsonEncoding encoding : JsonEncoding.values()) { + Charset charset = Charset.forName(encoding.getJavaName()); + ENCODINGS.put(charset, encoding); + } } @@ -103,7 +111,16 @@ public void setStreamingMediaTypes(List mediaTypes) { @Override public boolean canEncode(ResolvableType elementType, @Nullable MimeType mimeType) { Class clazz = elementType.toClass(); - return supportsMimeType(mimeType) && (Object.class == clazz || + if (!supportsMimeType(mimeType)) { + return false; + } + if (mimeType != null && mimeType.getCharset() != null) { + Charset charset = mimeType.getCharset(); + if (!ENCODINGS.containsKey(charset)) { + return false; + } + } + return (Object.class == clazz || (!String.class.isAssignableFrom(elementType.resolve(clazz)) && getObjectMapper().canSerialize(clazz))); } @@ -269,10 +286,9 @@ private byte[] streamSeparator(@Nullable MimeType mimeType) { protected JsonEncoding getJsonEncoding(@Nullable MimeType mimeType) { if (mimeType != null && mimeType.getCharset() != null) { Charset charset = mimeType.getCharset(); - for (JsonEncoding encoding : JsonEncoding.values()) { - if (charset.name().equals(encoding.getJavaName())) { - return encoding; - } + JsonEncoding result = ENCODINGS.get(charset); + if (result != null) { + return result; } } return JsonEncoding.UTF8; diff --git a/spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java b/spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java index 5e89310a25f2..363fd54cbe25 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java +++ b/spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java @@ -18,18 +18,13 @@ import java.lang.annotation.Annotation; 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; import java.util.List; import java.util.Map; -import java.util.function.Function; -import java.util.stream.Collectors; import com.fasterxml.jackson.annotation.JsonView; -import com.fasterxml.jackson.core.JsonEncoding; import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.type.TypeFactory; @@ -69,9 +64,6 @@ public abstract class Jackson2CodecSupport { new MimeType("application", "json", StandardCharsets.UTF_8), new MimeType("application", "*+json", StandardCharsets.UTF_8))); - private static final Map ENCODINGS = jsonEncodings(); - - protected final Log logger = HttpLogging.forLogName(getClass()); @@ -104,17 +96,7 @@ protected List getMimeTypes() { protected boolean supportsMimeType(@Nullable MimeType mimeType) { - if (mimeType == null) { - return true; - } - else if (this.mimeTypes.stream().noneMatch(m -> m.isCompatibleWith(mimeType))) { - return false; - } - else if (mimeType.getCharset() != null) { - Charset charset = mimeType.getCharset(); - return ENCODINGS.containsKey(charset.name()); - } - return true; + return (mimeType == null || this.mimeTypes.stream().anyMatch(m -> m.isCompatibleWith(mimeType))); } protected JavaType getJavaType(Type type, @Nullable Class contextClass) { @@ -143,10 +125,4 @@ protected MethodParameter getParameter(ResolvableType type) { @Nullable protected abstract A getAnnotation(MethodParameter parameter, Class annotType); - private static Map jsonEncodings() { - return EnumSet.allOf(JsonEncoding.class).stream() - .collect(Collectors.toMap(JsonEncoding::getJavaName, Function.identity())); - } - - } 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 bfb30140b71d..eb3d3bf9fc11 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 @@ -17,6 +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; @@ -37,6 +39,7 @@ 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; @@ -73,7 +76,7 @@ */ public abstract class AbstractJackson2HttpMessageConverter extends AbstractGenericHttpMessageConverter { - private static final Map ENCODINGS = jsonEncodings(); + private static final Map ENCODINGS = jsonEncodings(); /** * The default charset used by the converter. @@ -173,19 +176,17 @@ 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 causeRef = new AtomicReference<>(); if (this.objectMapper.canSerialize(clazz, causeRef)) { return true; @@ -194,14 +195,6 @@ 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. @@ -233,14 +226,6 @@ 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 { @@ -258,15 +243,31 @@ 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) { - return this.objectMapper.readerWithView(deserializationView).forType(javaType). - readValue(inputMessage.getBody()); + 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.readValue(inputMessage.getBody(), javaType); + if (isUnicode) { + return this.objectMapper.readValue(inputMessage.getBody(), javaType); + } + else { + Reader reader = new InputStreamReader(inputMessage.getBody(), charset); + return this.objectMapper.readValue(reader, javaType); + } } catch (InvalidDefinitionException ex) { throw new HttpMessageConversionException("Type definition error: " + ex.getType(), ex); @@ -276,6 +277,15 @@ 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 { @@ -363,7 +373,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.name()); + JsonEncoding encoding = ENCODINGS.get(charset); if (encoding != null) { return encoding; } @@ -388,9 +398,9 @@ protected Long getContentLength(Object object, @Nullable MediaType contentType) return super.getContentLength(object, contentType); } - private static Map jsonEncodings() { + private static Map jsonEncodings() { return EnumSet.allOf(JsonEncoding.class).stream() - .collect(Collectors.toMap(JsonEncoding::getJavaName, Function.identity())); + .collect(Collectors.toMap(encoding -> Charset.forName(encoding.getJavaName()), Function.identity())); } } diff --git a/spring-web/src/test/java/org/springframework/http/codec/json/Jackson2JsonDecoderTests.java b/spring-web/src/test/java/org/springframework/http/codec/json/Jackson2JsonDecoderTests.java index 9977ede10e2e..2e54321bdd67 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/json/Jackson2JsonDecoderTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/json/Jackson2JsonDecoderTests.java @@ -91,7 +91,7 @@ public void canDecode() { assertFalse(decoder.canDecode(forClass(Pojo.class), APPLICATION_XML)); assertTrue(this.decoder.canDecode(forClass(Pojo.class), new MediaType("application", "json", StandardCharsets.UTF_8))); - assertFalse(this.decoder.canDecode(forClass(Pojo.class), + assertTrue(this.decoder.canDecode(forClass(Pojo.class), new MediaType("application", "json", StandardCharsets.ISO_8859_1))); } @@ -239,6 +239,26 @@ public void decodeNonUtf8Encoding() { null); } + @Test + @SuppressWarnings("unchecked") + public void decodeNonUnicode() { + Flux input = Flux.concat( + stringBuffer("{\"føø\":\"bår\"}", StandardCharsets.ISO_8859_1) + ); + + testDecode(input, ResolvableType.forType(new ParameterizedTypeReference>() { + }), + step -> step.assertNext(o -> { + assertTrue(o instanceof Map); + Map map = (Map) o; + assertEquals(1, map.size()); + assertEquals("bår", map.get("føø")); + }) + .verifyComplete(), + MediaType.parseMediaType("application/json; charset=iso-8859-1"), + null); + } + @Test public void decodeMonoNonUtf8Encoding() { Mono input = stringBuffer("{\"foo\":\"bar\"}", StandardCharsets.UTF_16); diff --git a/spring-web/src/test/java/org/springframework/http/codec/json/Jackson2SmileDecoderTests.java b/spring-web/src/test/java/org/springframework/http/codec/json/Jackson2SmileDecoderTests.java index 7fb75ecb9deb..84ec4fcda694 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/json/Jackson2SmileDecoderTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/json/Jackson2SmileDecoderTests.java @@ -16,7 +16,6 @@ package org.springframework.http.codec.json; -import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; @@ -65,11 +64,6 @@ public void canDecode() { assertFalse(decoder.canDecode(forClass(String.class), null)); assertFalse(decoder.canDecode(forClass(Pojo.class), APPLICATION_JSON)); - - assertTrue(this.decoder.canDecode(ResolvableType.forClass(Pojo.class), - new MimeType("application", "x-jackson-smile", StandardCharsets.UTF_8))); - assertFalse(this.decoder.canDecode(ResolvableType.forClass(Pojo.class), - new MimeType("application", "x-jackson-smile", StandardCharsets.ISO_8859_1))); } diff --git a/spring-web/src/test/java/org/springframework/http/codec/json/Jackson2SmileEncoderTests.java b/spring-web/src/test/java/org/springframework/http/codec/json/Jackson2SmileEncoderTests.java index 0ff321ea3ba7..46e19e7bf659 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/json/Jackson2SmileEncoderTests.java +++ b/spring-web/src/test/java/org/springframework/http/codec/json/Jackson2SmileEncoderTests.java @@ -18,7 +18,6 @@ import java.io.IOException; import java.io.UncheckedIOException; -import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; @@ -71,11 +70,6 @@ public void canEncode() { assertTrue(this.encoder.canEncode(pojoType, STREAM_SMILE_MIME_TYPE)); assertTrue(this.encoder.canEncode(pojoType, null)); - assertTrue(this.encoder.canEncode(ResolvableType.forClass(Pojo.class), - new MimeType("application", "x-jackson-smile", StandardCharsets.UTF_8))); - assertFalse(this.encoder.canEncode(ResolvableType.forClass(Pojo.class), - new MimeType("application", "x-jackson-smile", StandardCharsets.ISO_8859_1))); - // SPR-15464 assertTrue(this.encoder.canEncode(ResolvableType.NONE, null)); } 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 3817d0635826..0bb83206ee87 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 @@ -18,6 +18,7 @@ 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; @@ -64,7 +65,7 @@ public void canRead() { assertTrue(converter.canRead(MyBean.class, new MediaType("application", "json"))); assertTrue(converter.canRead(Map.class, new MediaType("application", "json"))); assertTrue(converter.canRead(MyBean.class, new MediaType("application", "json", StandardCharsets.UTF_8))); - assertFalse(converter.canRead(MyBean.class, new MediaType("application", "json", StandardCharsets.ISO_8859_1))); + assertTrue(converter.canRead(MyBean.class, new MediaType("application", "json", StandardCharsets.ISO_8859_1))); } @Test @@ -439,7 +440,7 @@ public void writeSubTypeList() throws Exception { @Test public void readWithNoDefaultConstructor() throws Exception { String body = "{\"property1\":\"foo\",\"property2\":\"bar\"}"; - MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8")); + MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes(StandardCharsets.UTF_8)); inputMessage.getHeaders().setContentType(new MediaType("application", "json")); try { converter.read(BeanWithNoDefaultConstructor.class, inputMessage); @@ -451,6 +452,19 @@ public void readWithNoDefaultConstructor() throws Exception { fail(); } + @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 result = (HashMap) this.converter.read(HashMap.class, inputMessage); + + assertEquals(1, result.size()); + assertEquals("bår", result.get("føø")); + } + interface MyInterface { 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 88d8d7de0fdc..9860ae36ac97 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 @@ -49,8 +49,6 @@ public void canRead() { assertTrue(converter.canRead(MyBean.class, new MediaType("application", "x-jackson-smile"))); assertFalse(converter.canRead(MyBean.class, new MediaType("application", "json"))); assertFalse(converter.canRead(MyBean.class, new MediaType("application", "xml"))); - assertTrue(converter.canRead(MyBean.class, new MediaType("application", "x-jackson-smile", StandardCharsets.UTF_8))); - assertFalse(converter.canRead(MyBean.class, new MediaType("application", "x-jackson-smile", StandardCharsets.ISO_8859_1))); } @Test @@ -58,8 +56,6 @@ public void canWrite() { assertTrue(converter.canWrite(MyBean.class, new MediaType("application", "x-jackson-smile"))); assertFalse(converter.canWrite(MyBean.class, new MediaType("application", "json"))); assertFalse(converter.canWrite(MyBean.class, new MediaType("application", "xml"))); - assertTrue(converter.canWrite(MyBean.class, new MediaType("application", "x-jackson-smile", StandardCharsets.UTF_8))); - assertFalse(converter.canWrite(MyBean.class, new MediaType("application", "x-jackson-smile", StandardCharsets.ISO_8859_1))); } @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 01e79fdaac4a..e374a0555db5 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 @@ -17,6 +17,7 @@ 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; @@ -55,7 +56,7 @@ public void canRead() { assertTrue(converter.canRead(MyBean.class, new MediaType("text", "xml"))); assertTrue(converter.canRead(MyBean.class, new MediaType("application", "soap+xml"))); assertTrue(converter.canRead(MyBean.class, new MediaType("text", "xml", StandardCharsets.UTF_8))); - assertFalse(converter.canRead(MyBean.class, new MediaType("text", "xml", StandardCharsets.ISO_8859_1))); + assertTrue(converter.canRead(MyBean.class, new MediaType("text", "xml", StandardCharsets.ISO_8859_1))); } @Test @@ -194,6 +195,21 @@ public void readWithXmlBomb() throws IOException { this.converter.read(MyBean.class, inputMessage); } + @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", "xml", charset)); + MyBean result = (MyBean) converter.read(MyBean.class, inputMessage); + assertEquals("føø bår", result.getString()); + } + + public static class MyBean {