Skip to content

Commit f09900e

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. **Cherry-pick to `3.2.x`**
1 parent 3b1beab commit f09900e

File tree

2 files changed

+56
-58
lines changed

2 files changed

+56
-58
lines changed

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

Lines changed: 44 additions & 50 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.
@@ -119,7 +119,6 @@ protected AbstractJackson2MessageConverter(ObjectMapper objectMapper, MimeType c
119119
((DefaultJackson2JavaTypeMapper) this.javaTypeMapper).setTrustedPackages(trustedPackages);
120120
}
121121

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

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

@@ -311,7 +311,6 @@ public Object fromMessage(Message message, @Nullable Object conversionHint) thro
311311
Object content = null;
312312
MessageProperties properties = message.getMessageProperties();
313313
String contentType = properties.getContentType();
314-
// NOSONAR Boolean complexity
315314
if (this.assumeSupportedContentType && contentType.equals(MessageProperties.DEFAULT_CONTENT_TYPE)
316315
|| contentType.contains(this.supportedContentType.getSubtype())) {
317316

@@ -335,7 +334,7 @@ public Object fromMessage(Message message, @Nullable Object conversionHint) thro
335334
return content;
336335
}
337336

338-
private String determineEncoding(MessageProperties properties, @Nullable String contentType) {
337+
private @Nullable String determineEncoding(MessageProperties properties, @Nullable String contentType) {
339338
String encoding = properties.getContentEncoding();
340339
if (encoding == null && contentType != null) {
341340
try {
@@ -346,29 +345,23 @@ private String determineEncoding(MessageProperties properties, @Nullable String
346345
// Ignore
347346
}
348347
}
349-
if (encoding == null) {
350-
encoding = this.supportedCTCharset != null ? this.supportedCTCharset : getDefaultCharset();
351-
}
352348
return encoding;
353349
}
354350

355351
private Object doFromMessage(Message message, @Nullable Object conversionHint, MessageProperties properties,
356-
String encoding) {
352+
@Nullable String encoding) {
357353

358-
Object content = null;
359354
try {
360-
content = convertContent(message, conversionHint, properties, encoding);
355+
return convertContent(message, conversionHint, properties, encoding);
361356
}
362-
catch (IOException e) {
363-
throw new MessageConversionException(
364-
"Failed to convert Message content", e);
357+
catch (IOException ex) {
358+
throw new MessageConversionException("Failed to convert Message content", ex);
365359
}
366-
return content;
367360
}
368361

369362
@SuppressWarnings("NullAway") // Dataflow analysis limitation
370-
private Object convertContent(Message message, @Nullable Object conversionHint, MessageProperties properties, String encoding)
371-
throws IOException {
363+
private Object convertContent(Message message, @Nullable Object conversionHint, MessageProperties properties,
364+
@Nullable String encoding) throws IOException {
372365

373366
Object content = null;
374367
JavaType inferredType = this.javaTypeMapper.getInferredType(properties);
@@ -383,20 +376,15 @@ else if (inferredType != null && this.alwaysConvertToInferredType) {
383376
if (content == null) {
384377
if (conversionHint instanceof ParameterizedTypeReference<?> parameterizedTypeReference) {
385378
content = convertBytesToObject(message.getBody(), encoding,
386-
this.objectMapper.getTypeFactory().constructType(
387-
parameterizedTypeReference.getType()));
379+
this.objectMapper.getTypeFactory().constructType(parameterizedTypeReference.getType()));
388380
}
389381
else if (getClassMapper() == null) {
390-
JavaType targetJavaType = getJavaTypeMapper()
391-
.toJavaType(message.getMessageProperties());
392-
content = convertBytesToObject(message.getBody(),
393-
encoding, targetJavaType);
382+
JavaType targetJavaType = getJavaTypeMapper().toJavaType(message.getMessageProperties());
383+
content = convertBytesToObject(message.getBody(), encoding, targetJavaType);
394384
}
395385
else {
396-
Class<?> targetClass = getClassMapper().toClass(// NOSONAR never null
397-
message.getMessageProperties());
398-
content = convertBytesToObject(message.getBody(),
399-
encoding, targetClass);
386+
Class<?> targetClass = getClassMapper().toClass(message.getMessageProperties());
387+
content = convertBytesToObject(message.getBody(), encoding, targetClass);
400388
}
401389
}
402390
return content;
@@ -406,30 +394,36 @@ else if (getClassMapper() == null) {
406394
* Unfortunately, mapper.canDeserialize() always returns true (adds an AbstractDeserializer
407395
* to the cache); so all we can do is try a conversion.
408396
*/
409-
private @Nullable Object tryConvertType(Message message, String encoding, JavaType inferredType) {
397+
private @Nullable Object tryConvertType(Message message, @Nullable String encoding, JavaType inferredType) {
410398
try {
411399
return convertBytesToObject(message.getBody(), encoding, inferredType);
412400
}
413-
catch (Exception e) {
414-
this.log.trace("Cannot create possibly abstract container contents; falling back to headers", e);
401+
catch (Exception ex) {
402+
this.log.trace("Cannot create possibly abstract container contents; falling back to headers", ex);
415403
return null;
416404
}
417405
}
418406

419-
private Object convertBytesToObject(byte[] body, String encoding, JavaType targetJavaType) throws IOException {
420-
if (this.supportedCTCharset != null) { // Jackson will determine encoding
421-
return this.objectMapper.readValue(body, targetJavaType);
422-
}
423-
String contentAsString = new String(body, encoding);
424-
return this.objectMapper.readValue(contentAsString, targetJavaType);
407+
private Object convertBytesToObject(byte[] body, @Nullable String encoding, Class<?> targetClass)
408+
throws IOException {
409+
410+
return convertBytesToObject(body, encoding, this.objectMapper.constructType(targetClass));
425411
}
426412

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

435429
@Override
@@ -449,8 +443,7 @@ protected Message createMessage(Object objectToConvert, MessageProperties messag
449443
bytes = this.objectMapper.writeValueAsBytes(objectToConvert);
450444
}
451445
else {
452-
String jsonString = this.objectMapper
453-
.writeValueAsString(objectToConvert);
446+
String jsonString = this.objectMapper.writeValueAsString(objectToConvert);
454447
String encoding = this.supportedCTCharset != null ? this.supportedCTCharset : getDefaultCharset();
455448
bytes = jsonString.getBytes(encoding);
456449
}
@@ -465,16 +458,17 @@ protected Message createMessage(Object objectToConvert, MessageProperties messag
465458
messageProperties.setContentLength(bytes.length);
466459

467460
if (getClassMapper() == null) {
468-
JavaType type = this.objectMapper.constructType(
469-
genericType == null ? objectToConvert.getClass() : genericType);
461+
JavaType type =
462+
this.objectMapper.constructType(genericType == null ? objectToConvert.getClass() : genericType);
470463
if (genericType != null && !type.isContainerType()
471464
&& Modifier.isAbstract(type.getRawClass().getModifiers())) {
465+
472466
type = this.objectMapper.constructType(objectToConvert.getClass());
473467
}
474468
getJavaTypeMapper().fromJavaType(type, messageProperties);
475469
}
476470
else {
477-
getClassMapper().fromClass(objectToConvert.getClass(), messageProperties); // NOSONAR never null
471+
getClassMapper().fromClass(objectToConvert.getClass(), messageProperties);
478472
}
479473

480474
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);
@@ -458,8 +463,7 @@ void noConfigForCharsetInContentType() {
458463
assertThat(message2.getBody().length).isNotEqualTo(bodyLength8);
459464
converter.setDefaultCharset("UTF-8");
460465

461-
assertThatExceptionOfType(MessageConversionException.class).isThrownBy(
462-
() -> converter.fromMessage(message2));
466+
assertThat(converter.fromMessage(message2)).isEqualTo(trade);
463467
}
464468

465469
public List<Foo> fooLister() {

0 commit comments

Comments
 (0)