Handle Jackson InvalidDefinitionException with 5xx status in WebFlux

Issue: SPR-14925

Author: sdeleuze

spring-core/src/main/java/org/springframework/core/codec/InternalCodecException.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright 2002-2017 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.core.codec;
+
+/**
+ * Codec exception suitable for internal errors, like those not related to invalid data. It can be used to make sure
+ * such error will produce a 5xx status code and not a 4xx one when reading HTTP messages for example.
+ *
+ * @author Sebastien Deleuze
+ * @since 5.0
+ */
+@SuppressWarnings("serial")
+public class InternalCodecException extends CodecException {
+
+	public InternalCodecException(String msg) {
+		super(msg);
+	}
+
+	public InternalCodecException(String msg, Throwable cause) {
+		super(msg, cause);
+	}
+
+}

spring-web/src/main/java/org/springframework/http/codec/DecoderHttpMessageReader.java

@@ -21,7 +21,7 @@
 import java.util.List;
 import java.util.Map;
 
-import org.springframework.core.codec.CodecException;
+import org.springframework.core.codec.InternalCodecException;
 import org.springframework.http.HttpStatus;
 import org.springframework.web.server.ResponseStatusException;
 import reactor.core.publisher.Flux;
@@ -112,10 +112,10 @@ private Throwable mapError(Throwable ex) {
 		if (ex instanceof ResponseStatusException) {
 			return ex;
 		}
-		else if (ex instanceof CodecException) {
-			return new ResponseStatusException(HttpStatus.BAD_REQUEST, "Failed to decode HTTP message", ex);
+		else if (ex instanceof InternalCodecException) {
+			return new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Failed to decode HTTP message", ex);
 		}
-		return new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Failed to decode HTTP message", ex);
+		return new ResponseStatusException(HttpStatus.BAD_REQUEST, "Failed to decode HTTP message", ex);
 	}
 
 

spring-web/src/main/java/org/springframework/http/codec/json/Jackson2JsonDecoder.java

@@ -24,6 +24,7 @@
 import com.fasterxml.jackson.databind.JavaType;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectReader;
+import com.fasterxml.jackson.databind.exc.InvalidDefinitionException;
 import org.reactivestreams.Publisher;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
@@ -34,6 +35,7 @@
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.core.io.buffer.DataBufferUtils;
 import org.springframework.http.codec.HttpMessageDecoder;
+import org.springframework.core.codec.InternalCodecException;
 import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder;
 import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.http.server.reactive.ServerHttpResponse;
@@ -113,6 +115,9 @@ private Flux<Object> decodeInternal(JsonObjectDecoder objectDecoder, Publisher<D
 						DataBufferUtils.release(dataBuffer);
 						return value;
 					}
+					catch (InvalidDefinitionException ex) {
+						throw new InternalCodecException("Error while reading the data", ex);
+					}
 					catch (IOException ex) {
 						throw new CodecException("Error while reading the data", ex);
 					}

spring-web/src/test/java/org/springframework/http/codec/json/Jackson2JsonDecoderTests.java

@@ -33,6 +33,7 @@
 
 import org.springframework.core.ResolvableType;
 import org.springframework.core.codec.CodecException;
+import org.springframework.core.codec.InternalCodecException;
 import org.springframework.core.io.buffer.AbstractDataBufferAllocatingTestCase;
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.http.codec.Pojo;
@@ -160,4 +161,44 @@ public void decodeEmptyBodyToMono() throws Exception {
 				.verifyComplete();
 	}
 
+	@Test
+	public void invalidData() throws Exception {
+		Flux<DataBuffer> source = Flux.just(stringBuffer( "{\"property1\":\"foo\""));
+		ResolvableType elementType = forClass(BeanWithNoDefaultConstructor.class);
+		Flux<Object> flux = new Jackson2JsonDecoder().decode(source, elementType, null, emptyMap());
+		StepVerifier.create(flux).expectError(InternalCodecException.class);
+	}
+
+	@Test
+	public void noDefaultConstructor() throws Exception {
+		Flux<DataBuffer> source = Flux.just(stringBuffer( "{\"property1\":\"foo\",\"property2\":\"bar\"}"));
+		ResolvableType elementType = forClass(BeanWithNoDefaultConstructor.class);
+		Flux<Object> flux = new Jackson2JsonDecoder().decode(source, elementType, null, emptyMap());
+		StepVerifier
+				.create(flux)
+				.verifyErrorMatches(ex -> ex instanceof CodecException && !(ex instanceof InternalCodecException));
+	}
+
+
+	private static class BeanWithNoDefaultConstructor {
+
+		private final String property1;
+
+		private final String property2;
+
+		public BeanWithNoDefaultConstructor(String property1, String property2) {
+			this.property1 = property1;
+			this.property2 = property2;
+		}
+
+		public String getProperty1() {
+			return property1;
+		}
+
+		public String getProperty2() {
+			return property2;
+		}
+
+	}
+
 }

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageReaderArgumentResolver.java

@@ -23,6 +23,8 @@
 import java.util.stream.Collectors;
 
 import org.springframework.core.*;
+import org.springframework.core.codec.InternalCodecException;
+import org.springframework.http.HttpStatus;
 import org.springframework.web.server.ResponseStatusException;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
@@ -150,8 +152,12 @@ protected Mono<Object> readBody(MethodParameter bodyParameter, boolean isBodyReq
 		return Mono.error(new UnsupportedMediaTypeStatusException(mediaType, this.supportedMediaTypes));
 	}
 
-	private ServerWebInputException getReadError(MethodParameter parameter, Throwable ex) {
-		return new ServerWebInputException("Failed to read HTTP message", parameter, ex instanceof ResponseStatusException ? ex.getCause() : ex);
+	private ResponseStatusException getReadError(MethodParameter parameter, Throwable ex) {
+		Throwable cause = ex instanceof ResponseStatusException ? ex.getCause() : ex;
+
+		return cause instanceof InternalCodecException ?
+				new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Failed to read HTTP message", cause) :
+				new ServerWebInputException("Failed to read HTTP message", parameter, cause);
 	}
 
 	private ServerWebInputException getRequiredBodyError(MethodParameter parameter) {