Skip to content

Commit 28322d9

Browse files
artembilanartembilan
committed
GH-3038: Consistent handling of encoding in Jackson2MessageConverter
Fixes: #3038 Issue link: #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. # Conflicts: # spring-amqp/src/main/java/org/springframework/amqp/support/converter/AbstractJackson2MessageConverter.java
1 parent b73281a commit 28322d9

File tree

2 files changed

+64
-67
lines changed

2 files changed

+64
-67
lines changed

‎spring-amqp/src/main/java/org/springframework/amqp/support/converter/AbstractJackson2MessageConverter.java‎

Lines changed: 52 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,14 @@
5555
public abstract class AbstractJackson2MessageConverter extends AbstractMessageConverter
5656
implements BeanClassLoaderAware, SmartMessageConverter {
5757

58-
protected final Log log = LogFactory.getLog(getClass()); // NOSONAR protected
59-
6058
/**
6159
* The charset used when converting {@link String} to/from {@code byte[]}.
6260
*/
6361
public static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8;
6462

65-
protected final ObjectMapper objectMapper; // NOSONAR protected
63+
protected final Log log = LogFactory.getLog(getClass());
64+
65+
protected final ObjectMapper objectMapper;
6666

6767
/**
6868
* The supported content type; only the subtype is checked when decoding, e.g.
@@ -120,7 +120,6 @@ protected AbstractJackson2MessageConverter(ObjectMapper objectMapper, MimeType c
120120
((DefaultJackson2JavaTypeMapper) this.javaTypeMapper).setTrustedPackages(trustedPackages);
121121
}
122122

123-
124123
/**
125124
* Get the supported content type; only the subtype is checked when decoding, e.g.
126125
* */json, */xml. If this contains a charset parameter, when encoding, the
@@ -134,7 +133,6 @@ protected MimeType getSupportedContentType() {
134133
return this.supportedContentType;
135134
}
136135

137-
138136
/**
139137
* Set the supported content type; only the subtype is checked when decoding, e.g.
140138
* */json, */xml. If this contains a charset parameter, when encoding, the
@@ -175,8 +173,10 @@ public void setClassMapper(ClassMapper classMapper) {
175173
* @param defaultCharset The default charset.
176174
*/
177175
public void setDefaultCharset(@Nullable String defaultCharset) {
178-
this.defaultCharset = (defaultCharset != null) ? Charset.forName(defaultCharset)
179-
: DEFAULT_CHARSET;
176+
this.defaultCharset =
177+
defaultCharset != null
178+
? Charset.forName(defaultCharset)
179+
: DEFAULT_CHARSET;
180180
this.charsetIsUtf8 = this.defaultCharset.equals(StandardCharsets.UTF_8);
181181
}
182182

@@ -311,19 +311,17 @@ public Object fromMessage(Message message) throws MessageConversionException {
311311
public Object fromMessage(Message message, @Nullable Object conversionHint) throws MessageConversionException {
312312
Object content = null;
313313
MessageProperties properties = message.getMessageProperties();
314-
if (properties != null) {
315-
String contentType = properties.getContentType();
316-
if ((this.assumeSupportedContentType // NOSONAR Boolean complexity
317-
&& (contentType == null || contentType.equals(MessageProperties.DEFAULT_CONTENT_TYPE)))
318-
|| (contentType != null && contentType.contains(this.supportedContentType.getSubtype()))) {
319-
String encoding = determineEncoding(properties, contentType);
320-
content = doFromMessage(message, conversionHint, properties, encoding);
321-
}
322-
else {
323-
if (this.log.isWarnEnabled()) {
324-
this.log.warn("Could not convert incoming message with content-type ["
325-
+ contentType + "], '" + this.supportedContentType.getSubtype() + "' keyword missing.");
326-
}
314+
String contentType = properties.getContentType();
315+
if (this.assumeSupportedContentType && contentType.equals(MessageProperties.DEFAULT_CONTENT_TYPE)
316+
|| contentType.contains(this.supportedContentType.getSubtype())) {
317+
318+
String encoding = determineEncoding(properties, contentType);
319+
content = doFromMessage(message, conversionHint, properties, encoding);
320+
}
321+
else {
322+
if (this.log.isWarnEnabled()) {
323+
this.log.warn("Could not convert incoming message with content-type ["
324+
+ contentType + "], '" + this.supportedContentType.getSubtype() + "' keyword missing.");
327325
}
328326
}
329327
if (content == null) {
@@ -337,7 +335,7 @@ public Object fromMessage(Message message, @Nullable Object conversionHint) thro
337335
return content;
338336
}
339337

340-
private String determineEncoding(MessageProperties properties, @Nullable String contentType) {
338+
private @Nullable String determineEncoding(MessageProperties properties, @Nullable String contentType) {
341339
String encoding = properties.getContentEncoding();
342340
if (encoding == null && contentType != null) {
343341
try {
@@ -349,24 +347,18 @@ private String determineEncoding(MessageProperties properties, @Nullable String
349347
catch (RuntimeException e) {
350348
}
351349
}
352-
if (encoding == null) {
353-
encoding = this.supportedCTCharset != null ? this.supportedCTCharset : getDefaultCharset();
354-
}
355350
return encoding;
356351
}
357352

358353
private Object doFromMessage(Message message, Object conversionHint, MessageProperties properties,
359-
String encoding) {
354+
@Nullable String encoding) {
360355

361-
Object content = null;
362356
try {
363-
content = convertContent(message, conversionHint, properties, encoding);
357+
return convertContent(message, conversionHint, properties, encoding);
364358
}
365-
catch (IOException e) {
366-
throw new MessageConversionException(
367-
"Failed to convert Message content", e);
359+
catch (IOException ex) {
360+
throw new MessageConversionException("Failed to convert Message content", ex);
368361
}
369-
return content;
370362
}
371363

372364
private Object convertContent(Message message, Object conversionHint, MessageProperties properties, String encoding)
@@ -385,20 +377,15 @@ else if (inferredType != null && this.alwaysConvertToInferredType) {
385377
if (content == null) {
386378
if (conversionHint instanceof ParameterizedTypeReference<?> parameterizedTypeReference) {
387379
content = convertBytesToObject(message.getBody(), encoding,
388-
this.objectMapper.getTypeFactory().constructType(
389-
parameterizedTypeReference.getType()));
380+
this.objectMapper.getTypeFactory().constructType(parameterizedTypeReference.getType()));
390381
}
391382
else if (getClassMapper() == null) {
392-
JavaType targetJavaType = getJavaTypeMapper()
393-
.toJavaType(message.getMessageProperties());
394-
content = convertBytesToObject(message.getBody(),
395-
encoding, targetJavaType);
383+
JavaType targetJavaType = getJavaTypeMapper().toJavaType(message.getMessageProperties());
384+
content = convertBytesToObject(message.getBody(), encoding, targetJavaType);
396385
}
397386
else {
398-
Class<?> targetClass = getClassMapper().toClass(// NOSONAR never null
399-
message.getMessageProperties());
400-
content = convertBytesToObject(message.getBody(),
401-
encoding, targetClass);
387+
Class<?> targetClass = getClassMapper().toClass(message.getMessageProperties());
388+
content = convertBytesToObject(message.getBody(), encoding, targetClass);
402389
}
403390
}
404391
return content;
@@ -413,26 +400,32 @@ private Object tryConverType(Message message, String encoding, JavaType inferred
413400
try {
414401
return convertBytesToObject(message.getBody(), encoding, inferredType);
415402
}
416-
catch (Exception e) {
417-
this.log.trace("Cannot create possibly abstract container contents; falling back to headers", e);
403+
catch (Exception ex) {
404+
this.log.trace("Cannot create possibly abstract container contents; falling back to headers", ex);
418405
return null;
419406
}
420407
}
421408

422-
private Object convertBytesToObject(byte[] body, String encoding, JavaType targetJavaType) throws IOException {
423-
if (this.supportedCTCharset != null) { // Jackson will determine encoding
424-
return this.objectMapper.readValue(body, targetJavaType);
425-
}
426-
String contentAsString = new String(body, encoding);
427-
return this.objectMapper.readValue(contentAsString, targetJavaType);
409+
private Object convertBytesToObject(byte[] body, @Nullable String encoding, Class<?> targetClass)
410+
throws IOException {
411+
412+
return convertBytesToObject(body, encoding, this.objectMapper.constructType(targetClass));
428413
}
429414

430-
private Object convertBytesToObject(byte[] body, String encoding, Class<?> targetClass) throws IOException {
431-
if (this.supportedCTCharset != null) { // Jackson will determine encoding
432-
return this.objectMapper.readValue(body, this.objectMapper.constructType(targetClass));
415+
private Object convertBytesToObject(byte[] body, @Nullable String encoding, JavaType targetJavaType)
416+
throws IOException {
417+
418+
String encodingToUse = encoding;
419+
420+
if (encodingToUse == null) {
421+
if (this.charsetIsUtf8 & this.supportedCTCharset == null) {
422+
return this.objectMapper.readValue(body, targetJavaType);
423+
}
424+
encodingToUse = getDefaultCharset();
433425
}
434-
String contentAsString = new String(body, encoding);
435-
return this.objectMapper.readValue(contentAsString, this.objectMapper.constructType(targetClass));
426+
427+
String contentAsString = new String(body, encodingToUse);
428+
return this.objectMapper.readValue(contentAsString, targetJavaType);
436429
}
437430

438431
@Override
@@ -452,8 +445,7 @@ protected Message createMessage(Object objectToConvert, MessageProperties messag
452445
bytes = this.objectMapper.writeValueAsBytes(objectToConvert);
453446
}
454447
else {
455-
String jsonString = this.objectMapper
456-
.writeValueAsString(objectToConvert);
448+
String jsonString = this.objectMapper.writeValueAsString(objectToConvert);
457449
String encoding = this.supportedCTCharset != null ? this.supportedCTCharset : getDefaultCharset();
458450
bytes = jsonString.getBytes(encoding);
459451
}
@@ -468,16 +460,17 @@ protected Message createMessage(Object objectToConvert, MessageProperties messag
468460
messageProperties.setContentLength(bytes.length);
469461

470462
if (getClassMapper() == null) {
471-
JavaType type = this.objectMapper.constructType(
472-
genericType == null ? objectToConvert.getClass() : genericType);
463+
JavaType type =
464+
this.objectMapper.constructType(genericType == null ? objectToConvert.getClass() : genericType);
473465
if (genericType != null && !type.isContainerType()
474466
&& Modifier.isAbstract(type.getRawClass().getModifiers())) {
467+
475468
type = this.objectMapper.constructType(objectToConvert.getClass());
476469
}
477470
getJavaTypeMapper().fromJavaType(type, messageProperties);
478471
}
479472
else {
480-
getClassMapper().fromClass(objectToConvert.getClass(), messageProperties); // NOSONAR never null
473+
getClassMapper().fromClass(objectToConvert.getClass(), messageProperties);
481474
}
482475

483476
return new Message(bytes, messageProperties);

‎spring-amqp/src/test/java/org/springframework/amqp/support/converter/Jackson2JsonMessageConverterTests.java‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import org.springframework.util.MimeTypeUtils;
4444

4545
import static org.assertj.core.api.Assertions.assertThat;
46-
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
4746

4847
/**
4948
* @author Mark Pollack
@@ -154,14 +153,14 @@ public void shouldUseClassMapperWhenProvidedOutbound() {
154153

155154
@Test
156155
public void testAmqp330StringArray() {
157-
String[] testData = { "test" };
156+
String[] testData = {"test"};
158157
Message message = converter.toMessage(testData, new MessageProperties());
159158
assertThat((Object[]) converter.fromMessage(message)).isEqualTo(testData);
160159
}
161160

162161
@Test
163162
public void testAmqp330ObjectArray() {
164-
SimpleTrade[] testData = { trade };
163+
SimpleTrade[] testData = {trade};
165164
Message message = converter.toMessage(testData, new MessageProperties());
166165
assertThat((Object[]) converter.fromMessage(message)).isEqualTo(testData);
167166
}
@@ -229,7 +228,9 @@ public void testInferredGenericTypeInfo() throws Exception {
229228
byte[] bytes = "[ {\"name\" : \"foo\" } ]".getBytes();
230229
MessageProperties messageProperties = new MessageProperties();
231230
messageProperties.setContentType("application/json");
232-
messageProperties.setInferredArgumentType((new ParameterizedTypeReference<List<Foo>>() { }).getType());
231+
messageProperties.setInferredArgumentType((new ParameterizedTypeReference<List<Foo>>() {
232+
233+
}).getType());
233234
Message message = new Message(bytes, messageProperties);
234235
Object foo = this.converter.fromMessage(message);
235236
assertThat(foo).isInstanceOf(List.class);
@@ -242,7 +243,9 @@ public void testInferredGenericMap1() {
242243
MessageProperties messageProperties = new MessageProperties();
243244
messageProperties.setContentType("application/json");
244245
messageProperties.setInferredArgumentType(
245-
(new ParameterizedTypeReference<Map<String, List<Bar>>>() { }).getType());
246+
(new ParameterizedTypeReference<Map<String, List<Bar>>>() {
247+
248+
}).getType());
246249
Message message = new Message(bytes, messageProperties);
247250
Object foo = this.converter.fromMessage(message);
248251
assertThat(foo).isInstanceOf(LinkedHashMap.class);
@@ -260,7 +263,9 @@ public void testInferredGenericMap2() {
260263
MessageProperties messageProperties = new MessageProperties();
261264
messageProperties.setContentType("application/json");
262265
messageProperties.setInferredArgumentType(
263-
(new ParameterizedTypeReference<Map<String, Map<String, Bar>>>() { }).getType());
266+
(new ParameterizedTypeReference<Map<String, Map<String, Bar>>>() {
267+
268+
}).getType());
264269
Message message = new Message(bytes, messageProperties);
265270
Object foo = this.converter.fromMessage(message);
266271
assertThat(foo).isInstanceOf(LinkedHashMap.class);
@@ -459,8 +464,7 @@ void noConfigForCharsetInContentType() {
459464
assertThat(message2.getBody().length).isNotEqualTo(bodyLength8);
460465
converter.setDefaultCharset("UTF-8");
461466

462-
assertThatExceptionOfType(MessageConversionException.class).isThrownBy(
463-
() -> converter.fromMessage(message2));
467+
assertThat(converter.fromMessage(message2)).isEqualTo(trade);
464468
}
465469

466470
public List<Foo> fooLister() {

0 commit comments

Comments
 (0)