GH-1215: Allow Abstract Class Deserialization

Resolves #1215

Previously, the message converter would fall back to header type info
if the inferred type was abstract.

Furthermore, we did not examine container type content being abstract.

With a custom deserializer, abstract classes can be deserialized.

# Conflicts:
#	src/reference/asciidoc/whats-new.adoc

Author: garyrussell

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2019 the original author or authors.
+ * Copyright 2018-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -88,6 +88,8 @@ public abstract class AbstractJackson2MessageConverter extends AbstractMessageCo
 
 	private boolean assumeSupportedContentType = true;
 
+	private boolean alwaysConvertToInferredType;
+
 	/**
 	 * Construct with the provided {@link ObjectMapper} instance.
 	 * @param objectMapper the {@link ObjectMapper} to use.
@@ -201,6 +203,18 @@ public void setTypePrecedence(Jackson2JavaTypeMapper.TypePrecedence typePreceden
 		}
 	}
 
+	/**
+	 * When false (default), fall back to type id headers if the type (or contents of a container
+	 * type) is abstract. Set to true if conversion should always be attempted - perhaps because
+	 * a custom deserializer has been configured on the {@link ObjectMapper}. If the attempt fails,
+	 * fall back to headers.
+	 * @param alwaysAttemptConversion true to attempt.
+	 * @since 2.2.8
+	 */
+	public void setAlwaysConvertToInferredType(boolean alwaysAttemptConversion) {
+		this.alwaysConvertToInferredType = alwaysAttemptConversion;
+	}
+
 	protected boolean isUseProjectionForInterfaces() {
 		return this.useProjectionForInterfaces;
 	}
@@ -274,29 +288,34 @@ public Object fromMessage(Message message, @Nullable Object conversionHint) thro
 	private Object doFromMessage(Message message, Object conversionHint, MessageProperties properties,
 			String encoding) {
 
-		Object content;
+		Object content = null;
 		try {
 			JavaType inferredType = this.javaTypeMapper.getInferredType(properties);
 			if (inferredType != null && this.useProjectionForInterfaces && inferredType.isInterface()
 					&& !inferredType.getRawClass().getPackage().getName().startsWith("java.util")) { // List etc
 				content = this.projectingConverter.convert(message, inferredType.getRawClass());
 			}
-			else if (conversionHint instanceof ParameterizedTypeReference) {
-				content = convertBytesToObject(message.getBody(), encoding,
-						this.objectMapper.getTypeFactory().constructType(
-								((ParameterizedTypeReference<?>) conversionHint).getType()));
-			}
-			else if (getClassMapper() == null) {
-				JavaType targetJavaType = getJavaTypeMapper()
-						.toJavaType(message.getMessageProperties());
-				content = convertBytesToObject(message.getBody(),
-						encoding, targetJavaType);
+			else if (inferredType != null && this.alwaysConvertToInferredType) {
+				content = tryConverType(message, encoding, inferredType);
 			}
-			else {
-				Class<?> targetClass = getClassMapper().toClass(// NOSONAR never null
-						message.getMessageProperties());
-				content = convertBytesToObject(message.getBody(),
-						encoding, targetClass);
+			if (content == null) {
+				if (conversionHint instanceof ParameterizedTypeReference) {
+					content = convertBytesToObject(message.getBody(), encoding,
+							this.objectMapper.getTypeFactory().constructType(
+									((ParameterizedTypeReference<?>) conversionHint).getType()));
+				}
+				else if (getClassMapper() == null) {
+					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);
+				}
 			}
 		}
 		catch (IOException e) {
@@ -306,6 +325,21 @@ else if (getClassMapper() == null) {
 		return content;
 	}
 
+	/*
+	 * Unfortunately, mapper.canDeserialize() always returns true (adds an AbstractDeserializer
+	 * to the cache); so all we can do is try a conversion.
+	 */
+	@Nullable
+	private Object tryConverType(Message message, 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);
+			return null;
+		}
+	}
+
 	private Object convertBytesToObject(byte[] body, String encoding, JavaType targetJavaType) throws IOException {
 		String contentAsString = new String(body, encoding);
 		return this.objectMapper.readValue(contentAsString, targetJavaType);

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -112,9 +112,7 @@ public void addTrustedPackages(@Nullable String... packages) {
 	@Override
 	public JavaType toJavaType(MessageProperties properties) {
 		JavaType inferredType = getInferredType(properties);
-		if (inferredType != null
-			 && ((!inferredType.isAbstract() && !inferredType.isInterface()
-					|| inferredType.getRawClass().getPackage().getName().startsWith("java.util")))) {
+		if (inferredType != null && canConvert(inferredType)) {
 			return inferredType;
 		}
 
@@ -131,6 +129,19 @@ public JavaType toJavaType(MessageProperties properties) {
 		return TypeFactory.defaultInstance().constructType(Object.class);
 	}
 
+	private boolean canConvert(JavaType inferredType) {
+		if (inferredType.isAbstract()) {
+			return false;
+		}
+		if (inferredType.isContainerType() && inferredType.getContentType().isAbstract()) {
+			return false;
+		}
+		if (inferredType.getKeyType() != null && inferredType.getKeyType().isAbstract()) {
+			return false;
+		}
+		return true;
+	}
+
 	private JavaType fromTypeHeader(MessageProperties properties, String typeIdHeader) {
 		JavaType classType = getClassIdType(typeIdHeader);
 		if (!classType.isContainerType() || classType.isArrayType()) {
@@ -151,7 +162,7 @@ private JavaType fromTypeHeader(MessageProperties properties, String typeIdHeade
 	@Override
 	@Nullable
 	public JavaType getInferredType(MessageProperties properties) {
-		if (hasInferredTypeHeader(properties) && this.typePrecedence.equals(TypePrecedence.INFERRED)) {
+		if (this.typePrecedence.equals(TypePrecedence.INFERRED) && hasInferredTypeHeader(properties)) {
 			return fromInferredTypeHeader(properties);
 		}
 		return null;

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.io.IOException;
 import java.math.BigDecimal;
 import java.util.Hashtable;
 import java.util.LinkedHashMap;
@@ -34,7 +35,12 @@
 import org.springframework.data.web.JsonPath;
 import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
 
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
+import com.fasterxml.jackson.databind.module.SimpleModule;
 import com.fasterxml.jackson.databind.ser.BeanSerializerFactory;
 
 /**
@@ -299,6 +305,75 @@ public void testMissingContentType() {
 		assertThat(foo).isSameAs(bytes);
 	}
 
+	@Test
+	void customAbstractClass() {
+		byte[] bytes = "{\"field\" : \"foo\" }".getBytes();
+		MessageProperties messageProperties = new MessageProperties();
+		messageProperties.setHeader("__TypeId__", String.class.getName());
+		messageProperties.setInferredArgumentType(Baz.class);
+		Message message = new Message(bytes, messageProperties);
+		ObjectMapper mapper = new ObjectMapper();
+		mapper.registerModule(new BazModule());
+		Jackson2JsonMessageConverter j2Converter = new Jackson2JsonMessageConverter(mapper);
+		j2Converter.setAlwaysConvertToInferredType(true);
+		Baz baz = (Baz) j2Converter.fromMessage(message);
+		assertThat(((Qux) baz).getField()).isEqualTo("foo");
+	}
+
+	@Test
+	void fallbackToHeaders() {
+		byte[] bytes = "{\"field\" : \"foo\" }".getBytes();
+		MessageProperties messageProperties = new MessageProperties();
+		messageProperties.setHeader("__TypeId__", Buz.class.getName());
+		messageProperties.setInferredArgumentType(Baz.class);
+		Message message = new Message(bytes, messageProperties);
+		Jackson2JsonMessageConverter j2Converter = new Jackson2JsonMessageConverter();
+		Fiz buz = (Fiz) j2Converter.fromMessage(message);
+		assertThat(((Buz) buz).getField()).isEqualTo("foo");
+	}
+
+	@Test
+	void customAbstractClassList() throws Exception {
+		byte[] bytes = "[{\"field\" : \"foo\" }]".getBytes();
+		MessageProperties messageProperties = new MessageProperties();
+		messageProperties.setHeader("__TypeId__", String.class.getName());
+		messageProperties.setInferredArgumentType(getClass().getDeclaredMethod("bazLister").getGenericReturnType());
+		Message message = new Message(bytes, messageProperties);
+		ObjectMapper mapper = new ObjectMapper();
+		mapper.registerModule(new BazModule());
+		Jackson2JsonMessageConverter j2Converter = new Jackson2JsonMessageConverter(mapper);
+		j2Converter.setAlwaysConvertToInferredType(true);
+		@SuppressWarnings("unchecked")
+		List<Baz> bazs = (List<Baz>) j2Converter.fromMessage(message);
+		assertThat(bazs).hasSize(1);
+		assertThat(((Qux) bazs.get(0)).getField()).isEqualTo("foo");
+	}
+
+	@Test
+	void cantDeserializeFizListUseHeaders() throws Exception {
+		byte[] bytes = "[{\"field\" : \"foo\" }]".getBytes();
+		MessageProperties messageProperties = new MessageProperties();
+		messageProperties.setInferredArgumentType(getClass().getDeclaredMethod("fizLister").getGenericReturnType());
+		messageProperties.setHeader("__TypeId__", List.class.getName());
+		messageProperties.setHeader("__ContentTypeId__", Buz.class.getName());
+		Message message = new Message(bytes, messageProperties);
+		ObjectMapper mapper = new ObjectMapper();
+		mapper.registerModule(new BazModule());
+		Jackson2JsonMessageConverter j2Converter = new Jackson2JsonMessageConverter(mapper);
+		@SuppressWarnings("unchecked")
+		List<Fiz> buzs = (List<Fiz>) j2Converter.fromMessage(message);
+		assertThat(buzs).hasSize(1);
+		assertThat(((Buz) buzs.get(0)).getField()).isEqualTo("foo");
+	}
+
+	public List<Baz> bazLister() {
+		return null;
+	}
+
+	public List<Fiz> fizLister() {
+		return null;
+	}
+
 	public static class Foo {
 
 		private String name = "foo";
@@ -424,4 +499,73 @@ interface Sample {
 
 	}
 
+	public interface Baz {
+
+	}
+
+	public static class Qux implements Baz {
+
+		private String field;
+
+		public Qux(String field) {
+			this.field = field;
+		}
+
+		public String getField() {
+			return this.field;
+		}
+
+		public void setField(String field) {
+			this.field = field;
+		}
+
+	}
+
+	@SuppressWarnings("serial")
+	public static class BazDeserializer extends StdDeserializer<Baz> {
+
+		public BazDeserializer() {
+			super(Baz.class);
+		}
+
+		@Override
+		public Baz deserialize(JsonParser p, DeserializationContext ctxt)
+				throws IOException, JsonProcessingException {
+
+			p.nextFieldName();
+			String field = p.nextTextValue();
+			p.nextToken();
+			return new Qux(field);
+
+		}
+
+	}
+
+	public interface Fiz {
+
+	}
+
+	public static class Buz implements Fiz {
+
+		private String field;
+
+		public String getField() {
+			return this.field;
+		}
+
+		public void setField(String field) {
+			this.field = field;
+		}
+
+	}
+
+	@SuppressWarnings("serial")
+	public static class BazModule extends SimpleModule {
+
+		public BazModule() {
+			addDeserializer(Baz.class, new BazDeserializer());
+		}
+
+	}
+
 }

src/reference/asciidoc/amqp.adoc

@@ -3553,6 +3553,15 @@ converter to determine the type.
 IMPORTANT: Starting with version 1.6.11, `Jackson2JsonMessageConverter` and, therefore, `DefaultJackson2JavaTypeMapper` (`DefaultClassMapper`) provide the `trustedPackages` option to overcome https://pivotal.io/security/cve-2017-4995[Serialization Gadgets] vulnerability.
 By default and for backward compatibility, the `Jackson2JsonMessageConverter` trusts all packages -- that is, it uses `*` for the option.
 
+[[jackson-abstract]]
+====== Deserializing Abstract Classes
+
+Prior to version 2.2.8, if the inferred type of a `@RabbitListener` was an abstract class (including interfaces), the converter would fall back to looking for type information in the headers and, if present, used that information; if that was not present, it would try to create the abstract class.
+This caused a problem when a custom `ObjectMapper` that is configured with a custom deserializer to handle the abstract class is used, but the incoming message has invalid type headers.
+
+Starting with version 2.2.8, the previous behavior is retained by default. If you have such a custom `ObjectMapper` and you want to ignore type headers, and always use the inferred type for conversion, set the `alwaysConvertToInferredType` to `true`.
+This is needed for backwards compatibility and to avoid the overhead of an attempted conversion when it would fail (with a standard `ObjectMapper`).
+
 [[data-projection]]
 ====== Using Spring Data Projection Interfaces
 

src/reference/asciidoc/whats-new.adoc

@@ -96,6 +96,8 @@ The `CachingConnectionFactory` has a new property `shuffleAddresses`.
 When providing a list of broker node addresses, the list will be shuffled before creating a connection so that the order in which the connections are attempted is random.
 See <<cluster>> for more information.
 
+==== Testing Changes
+
 When using Publisher confirms and returns, the callbacks are now invoked on the connection factory's `executor`.
 This avoids a possible deadlock in the `amqp-clients` library if you perform rabbit operations from within the callback.
 See <<template-confirms>> for more information.