GH-3038: Consistent handling of encoding in Jackson2MessageConverter
Fixes: #3038 Issue link: https://github.com/spring-projects/spring-amqp/issues/3038 The `AbstractJackson2MessageConverter.doFromMessage()` falls back to the default encoding when no encoding in the message properties. However, that should be handled as `objectMapper.readValue(byte[])` when no `supportedCTCharset` and `charsetIsUtf8`, not converted to String before with that default encoding. **Cherry-pick to `3.2.x`**
This commit is contained in:
@@ -55,14 +55,14 @@ import org.springframework.util.MimeTypeUtils;
|
||||
public abstract class AbstractJackson2MessageConverter extends AbstractMessageConverter
|
||||
implements BeanClassLoaderAware, SmartMessageConverter {
|
||||
|
||||
protected final Log log = LogFactory.getLog(getClass()); // NOSONAR protected
|
||||
|
||||
/**
|
||||
* The charset used when converting {@link String} to/from {@code byte[]}.
|
||||
*/
|
||||
public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8;
|
||||
|
||||
protected final ObjectMapper objectMapper; // NOSONAR protected
|
||||
protected final Log log = LogFactory.getLog(getClass());
|
||||
|
||||
protected final ObjectMapper objectMapper;
|
||||
|
||||
/**
|
||||
* The supported content type; only the subtype is checked when decoding, e.g.
|
||||
@@ -119,7 +119,6 @@ public abstract class AbstractJackson2MessageConverter extends AbstractMessageCo
|
||||
((DefaultJackson2JavaTypeMapper) this.javaTypeMapper).setTrustedPackages(trustedPackages);
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Get the supported content type; only the subtype is checked when decoding, e.g.
|
||||
* */json, */xml. If this contains a charset parameter, when encoding, the
|
||||
@@ -133,7 +132,6 @@ public abstract class AbstractJackson2MessageConverter extends AbstractMessageCo
|
||||
return this.supportedContentType;
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Set the supported content type; only the subtype is checked when decoding, e.g.
|
||||
* */json, */xml. If this contains a charset parameter, when encoding, the
|
||||
@@ -174,8 +172,10 @@ public abstract class AbstractJackson2MessageConverter extends AbstractMessageCo
|
||||
* @param defaultCharset The default charset.
|
||||
*/
|
||||
public void setDefaultCharset(@Nullable String defaultCharset) {
|
||||
this.defaultCharset = (defaultCharset != null) ? Charset.forName(defaultCharset)
|
||||
: DEFAULT_CHARSET;
|
||||
this.defaultCharset =
|
||||
defaultCharset != null
|
||||
? Charset.forName(defaultCharset)
|
||||
: DEFAULT_CHARSET;
|
||||
this.charsetIsUtf8 = this.defaultCharset.equals(StandardCharsets.UTF_8);
|
||||
}
|
||||
|
||||
@@ -311,7 +311,6 @@ public abstract class AbstractJackson2MessageConverter extends AbstractMessageCo
|
||||
Object content = null;
|
||||
MessageProperties properties = message.getMessageProperties();
|
||||
String contentType = properties.getContentType();
|
||||
// NOSONAR Boolean complexity
|
||||
if (this.assumeSupportedContentType && contentType.equals(MessageProperties.DEFAULT_CONTENT_TYPE)
|
||||
|| contentType.contains(this.supportedContentType.getSubtype())) {
|
||||
|
||||
@@ -335,7 +334,7 @@ public abstract class AbstractJackson2MessageConverter extends AbstractMessageCo
|
||||
return content;
|
||||
}
|
||||
|
||||
private String determineEncoding(MessageProperties properties, @Nullable String contentType) {
|
||||
private @Nullable String determineEncoding(MessageProperties properties, @Nullable String contentType) {
|
||||
String encoding = properties.getContentEncoding();
|
||||
if (encoding == null && contentType != null) {
|
||||
try {
|
||||
@@ -346,29 +345,23 @@ public abstract class AbstractJackson2MessageConverter extends AbstractMessageCo
|
||||
// Ignore
|
||||
}
|
||||
}
|
||||
if (encoding == null) {
|
||||
encoding = this.supportedCTCharset != null ? this.supportedCTCharset : getDefaultCharset();
|
||||
}
|
||||
return encoding;
|
||||
}
|
||||
|
||||
private Object doFromMessage(Message message, @Nullable Object conversionHint, MessageProperties properties,
|
||||
String encoding) {
|
||||
@Nullable String encoding) {
|
||||
|
||||
Object content = null;
|
||||
try {
|
||||
content = convertContent(message, conversionHint, properties, encoding);
|
||||
return convertContent(message, conversionHint, properties, encoding);
|
||||
}
|
||||
catch (IOException e) {
|
||||
throw new MessageConversionException(
|
||||
"Failed to convert Message content", e);
|
||||
catch (IOException ex) {
|
||||
throw new MessageConversionException("Failed to convert Message content", ex);
|
||||
}
|
||||
return content;
|
||||
}
|
||||
|
||||
@SuppressWarnings("NullAway") // Dataflow analysis limitation
|
||||
private Object convertContent(Message message, @Nullable Object conversionHint, MessageProperties properties, String encoding)
|
||||
throws IOException {
|
||||
private Object convertContent(Message message, @Nullable Object conversionHint, MessageProperties properties,
|
||||
@Nullable String encoding) throws IOException {
|
||||
|
||||
Object content = null;
|
||||
JavaType inferredType = this.javaTypeMapper.getInferredType(properties);
|
||||
@@ -383,20 +376,15 @@ public abstract class AbstractJackson2MessageConverter extends AbstractMessageCo
|
||||
if (content == null) {
|
||||
if (conversionHint instanceof ParameterizedTypeReference<?> parameterizedTypeReference) {
|
||||
content = convertBytesToObject(message.getBody(), encoding,
|
||||
this.objectMapper.getTypeFactory().constructType(
|
||||
parameterizedTypeReference.getType()));
|
||||
this.objectMapper.getTypeFactory().constructType(parameterizedTypeReference.getType()));
|
||||
}
|
||||
else if (getClassMapper() == null) {
|
||||
JavaType targetJavaType = getJavaTypeMapper()
|
||||
.toJavaType(message.getMessageProperties());
|
||||
content = convertBytesToObject(message.getBody(),
|
||||
encoding, targetJavaType);
|
||||
JavaType targetJavaType = getJavaTypeMapper().toJavaType(message.getMessageProperties());
|
||||
content = convertBytesToObject(message.getBody(), encoding, targetJavaType);
|
||||
}
|
||||
else {
|
||||
Class<?> targetClass = getClassMapper().toClass(// NOSONAR never null
|
||||
message.getMessageProperties());
|
||||
content = convertBytesToObject(message.getBody(),
|
||||
encoding, targetClass);
|
||||
Class<?> targetClass = getClassMapper().toClass(message.getMessageProperties());
|
||||
content = convertBytesToObject(message.getBody(), encoding, targetClass);
|
||||
}
|
||||
}
|
||||
return content;
|
||||
@@ -406,30 +394,36 @@ public abstract class AbstractJackson2MessageConverter extends AbstractMessageCo
|
||||
* Unfortunately, mapper.canDeserialize() always returns true (adds an AbstractDeserializer
|
||||
* to the cache); so all we can do is try a conversion.
|
||||
*/
|
||||
private @Nullable Object tryConvertType(Message message, String encoding, JavaType inferredType) {
|
||||
private @Nullable Object tryConvertType(Message message, @Nullable String encoding, JavaType inferredType) {
|
||||
try {
|
||||
return convertBytesToObject(message.getBody(), encoding, inferredType);
|
||||
}
|
||||
catch (Exception e) {
|
||||
this.log.trace("Cannot create possibly abstract container contents; falling back to headers", e);
|
||||
catch (Exception ex) {
|
||||
this.log.trace("Cannot create possibly abstract container contents; falling back to headers", ex);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
private Object convertBytesToObject(byte[] body, String encoding, JavaType targetJavaType) throws IOException {
|
||||
if (this.supportedCTCharset != null) { // Jackson will determine encoding
|
||||
return this.objectMapper.readValue(body, targetJavaType);
|
||||
}
|
||||
String contentAsString = new String(body, encoding);
|
||||
return this.objectMapper.readValue(contentAsString, targetJavaType);
|
||||
private Object convertBytesToObject(byte[] body, @Nullable String encoding, Class<?> targetClass)
|
||||
throws IOException {
|
||||
|
||||
return convertBytesToObject(body, encoding, this.objectMapper.constructType(targetClass));
|
||||
}
|
||||
|
||||
private Object convertBytesToObject(byte[] body, String encoding, Class<?> targetClass) throws IOException {
|
||||
if (this.supportedCTCharset != null) { // Jackson will determine encoding
|
||||
return this.objectMapper.readValue(body, this.objectMapper.constructType(targetClass));
|
||||
private Object convertBytesToObject(byte[] body, @Nullable String encoding, JavaType targetJavaType)
|
||||
throws IOException {
|
||||
|
||||
String encodingToUse = encoding;
|
||||
|
||||
if (encodingToUse == null) {
|
||||
if (this.charsetIsUtf8 & this.supportedCTCharset == null) {
|
||||
return this.objectMapper.readValue(body, targetJavaType);
|
||||
}
|
||||
encodingToUse = getDefaultCharset();
|
||||
}
|
||||
String contentAsString = new String(body, encoding);
|
||||
return this.objectMapper.readValue(contentAsString, this.objectMapper.constructType(targetClass));
|
||||
|
||||
String contentAsString = new String(body, encodingToUse);
|
||||
return this.objectMapper.readValue(contentAsString, targetJavaType);
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -449,8 +443,7 @@ public abstract class AbstractJackson2MessageConverter extends AbstractMessageCo
|
||||
bytes = this.objectMapper.writeValueAsBytes(objectToConvert);
|
||||
}
|
||||
else {
|
||||
String jsonString = this.objectMapper
|
||||
.writeValueAsString(objectToConvert);
|
||||
String jsonString = this.objectMapper.writeValueAsString(objectToConvert);
|
||||
String encoding = this.supportedCTCharset != null ? this.supportedCTCharset : getDefaultCharset();
|
||||
bytes = jsonString.getBytes(encoding);
|
||||
}
|
||||
@@ -465,16 +458,17 @@ public abstract class AbstractJackson2MessageConverter extends AbstractMessageCo
|
||||
messageProperties.setContentLength(bytes.length);
|
||||
|
||||
if (getClassMapper() == null) {
|
||||
JavaType type = this.objectMapper.constructType(
|
||||
genericType == null ? objectToConvert.getClass() : genericType);
|
||||
JavaType type =
|
||||
this.objectMapper.constructType(genericType == null ? objectToConvert.getClass() : genericType);
|
||||
if (genericType != null && !type.isContainerType()
|
||||
&& Modifier.isAbstract(type.getRawClass().getModifiers())) {
|
||||
|
||||
type = this.objectMapper.constructType(objectToConvert.getClass());
|
||||
}
|
||||
getJavaTypeMapper().fromJavaType(type, messageProperties);
|
||||
}
|
||||
else {
|
||||
getClassMapper().fromClass(objectToConvert.getClass(), messageProperties); // NOSONAR never null
|
||||
getClassMapper().fromClass(objectToConvert.getClass(), messageProperties);
|
||||
}
|
||||
|
||||
return new Message(bytes, messageProperties);
|
||||
|
||||
@@ -43,7 +43,6 @@ import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
|
||||
import org.springframework.util.MimeTypeUtils;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
|
||||
|
||||
/**
|
||||
* @author Mark Pollack
|
||||
@@ -154,14 +153,14 @@ public class Jackson2JsonMessageConverterTests {
|
||||
|
||||
@Test
|
||||
public void testAmqp330StringArray() {
|
||||
String[] testData = { "test" };
|
||||
String[] testData = {"test"};
|
||||
Message message = converter.toMessage(testData, new MessageProperties());
|
||||
assertThat((Object[]) converter.fromMessage(message)).isEqualTo(testData);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testAmqp330ObjectArray() {
|
||||
SimpleTrade[] testData = { trade };
|
||||
SimpleTrade[] testData = {trade};
|
||||
Message message = converter.toMessage(testData, new MessageProperties());
|
||||
assertThat((Object[]) converter.fromMessage(message)).isEqualTo(testData);
|
||||
}
|
||||
@@ -229,7 +228,9 @@ public class Jackson2JsonMessageConverterTests {
|
||||
byte[] bytes = "[ {\"name\" : \"foo\" } ]".getBytes();
|
||||
MessageProperties messageProperties = new MessageProperties();
|
||||
messageProperties.setContentType("application/json");
|
||||
messageProperties.setInferredArgumentType((new ParameterizedTypeReference<List<Foo>>() { }).getType());
|
||||
messageProperties.setInferredArgumentType((new ParameterizedTypeReference<List<Foo>>() {
|
||||
|
||||
}).getType());
|
||||
Message message = new Message(bytes, messageProperties);
|
||||
Object foo = this.converter.fromMessage(message);
|
||||
assertThat(foo).isInstanceOf(List.class);
|
||||
@@ -242,7 +243,9 @@ public class Jackson2JsonMessageConverterTests {
|
||||
MessageProperties messageProperties = new MessageProperties();
|
||||
messageProperties.setContentType("application/json");
|
||||
messageProperties.setInferredArgumentType(
|
||||
(new ParameterizedTypeReference<Map<String, List<Bar>>>() { }).getType());
|
||||
(new ParameterizedTypeReference<Map<String, List<Bar>>>() {
|
||||
|
||||
}).getType());
|
||||
Message message = new Message(bytes, messageProperties);
|
||||
Object foo = this.converter.fromMessage(message);
|
||||
assertThat(foo).isInstanceOf(LinkedHashMap.class);
|
||||
@@ -260,7 +263,9 @@ public class Jackson2JsonMessageConverterTests {
|
||||
MessageProperties messageProperties = new MessageProperties();
|
||||
messageProperties.setContentType("application/json");
|
||||
messageProperties.setInferredArgumentType(
|
||||
(new ParameterizedTypeReference<Map<String, Map<String, Bar>>>() { }).getType());
|
||||
(new ParameterizedTypeReference<Map<String, Map<String, Bar>>>() {
|
||||
|
||||
}).getType());
|
||||
Message message = new Message(bytes, messageProperties);
|
||||
Object foo = this.converter.fromMessage(message);
|
||||
assertThat(foo).isInstanceOf(LinkedHashMap.class);
|
||||
@@ -458,8 +463,7 @@ public class Jackson2JsonMessageConverterTests {
|
||||
assertThat(message2.getBody().length).isNotEqualTo(bodyLength8);
|
||||
converter.setDefaultCharset("UTF-8");
|
||||
|
||||
assertThatExceptionOfType(MessageConversionException.class).isThrownBy(
|
||||
() -> converter.fromMessage(message2));
|
||||
assertThat(converter.fromMessage(message2)).isEqualTo(trade);
|
||||
}
|
||||
|
||||
public List<Foo> fooLister() {
|
||||
|
||||
Reference in New Issue
Block a user