From ed8565894a4780716d238a9ae32910020730d0ce Mon Sep 17 00:00:00 2001 From: Sebastien Deleuze Date: Wed, 5 Apr 2017 10:36:01 +0200 Subject: [PATCH] Return 5xx HTTP status for invalid target types with Jackson InvalidDefinitionException has been introduced in Jackson 2.9 to be able to differentiate invalid data sent from the client (should still generate a 4xx HTTP status code) from server side errors like beans with no default constructor (should generate a 5xx HTTP status code). Issue: SPR-14925 --- .../HttpMessageConversionException.java | 26 +++++++++++++ .../HttpMessageNotReadableException.java | 13 +++++++ .../AbstractJackson2HttpMessageConverter.java | 5 +++ ...pingJackson2HttpMessageConverterTests.java | 39 +++++++++++++++++++ .../DefaultHandlerExceptionResolver.java | 3 +- 5 files changed, 85 insertions(+), 1 deletion(-) diff --git a/spring-web/src/main/java/org/springframework/http/converter/HttpMessageConversionException.java b/spring-web/src/main/java/org/springframework/http/converter/HttpMessageConversionException.java index 413f3075b92..b44ad6a5b5e 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/HttpMessageConversionException.java +++ b/spring-web/src/main/java/org/springframework/http/converter/HttpMessageConversionException.java @@ -17,22 +17,29 @@ package org.springframework.http.converter; import org.springframework.core.NestedRuntimeException; +import org.springframework.http.HttpStatus; + +import java.util.Optional; /** * Thrown by {@link HttpMessageConverter} implementations when a conversion attempt fails. * * @author Arjen Poutsma + * @author Sebastien Deleuze * @since 3.0 */ @SuppressWarnings("serial") public class HttpMessageConversionException extends NestedRuntimeException { + private final HttpStatus errorStatus; + /** * Create a new HttpMessageConversionException. * @param msg the detail message */ public HttpMessageConversionException(String msg) { super(msg); + this.errorStatus = null; } /** @@ -42,6 +49,25 @@ public class HttpMessageConversionException extends NestedRuntimeException { */ public HttpMessageConversionException(String msg, Throwable cause) { super(msg, cause); + this.errorStatus = null; } + /** + * Create a new HttpMessageConversionException. + * @since 5.0 + * @param msg the detail message + * @param cause the root cause (if any) + * @param errorStatus the HTTP error status related to this exception + */ + public HttpMessageConversionException(String msg, Throwable cause, HttpStatus errorStatus) { + super(msg, cause); + this.errorStatus = errorStatus; + } + + /** + * Return the HTTP error status related to this exception if any. + */ + public Optional getErrorStatus() { + return Optional.ofNullable(errorStatus); + } } diff --git a/spring-web/src/main/java/org/springframework/http/converter/HttpMessageNotReadableException.java b/spring-web/src/main/java/org/springframework/http/converter/HttpMessageNotReadableException.java index 8d35ac90c3d..5ac2c6a351f 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/HttpMessageNotReadableException.java +++ b/spring-web/src/main/java/org/springframework/http/converter/HttpMessageNotReadableException.java @@ -16,6 +16,8 @@ package org.springframework.http.converter; +import org.springframework.http.HttpStatus; + /** * Thrown by {@link HttpMessageConverter} implementations when the * {@link HttpMessageConverter#read} method fails. @@ -43,4 +45,15 @@ public class HttpMessageNotReadableException extends HttpMessageConversionExcept super(msg, cause); } + /** + * Create a new HttpMessageNotReadableException. + * @since 5.0 + * @param msg the detail message + * @param cause the root cause (if any) + * @param errorStatus the HTTP error status related to this exception + */ + public HttpMessageNotReadableException(String msg, Throwable cause, HttpStatus errorStatus) { + super(msg, cause, errorStatus); + } + } diff --git a/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java index 8785de48d0c..a2dbcbda1ab 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/json/AbstractJackson2HttpMessageConverter.java @@ -36,12 +36,14 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectWriter; import com.fasterxml.jackson.databind.SerializationConfig; import com.fasterxml.jackson.databind.SerializationFeature; +import com.fasterxml.jackson.databind.exc.InvalidDefinitionException; import com.fasterxml.jackson.databind.ser.FilterProvider; import com.fasterxml.jackson.databind.type.TypeFactory; import org.springframework.core.ResolvableType; import org.springframework.http.HttpInputMessage; import org.springframework.http.HttpOutputMessage; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.converter.AbstractGenericHttpMessageConverter; import org.springframework.http.converter.HttpMessageConverter; @@ -235,6 +237,9 @@ public abstract class AbstractJackson2HttpMessageConverter extends AbstractGener } return this.objectMapper.readValue(inputMessage.getBody(), javaType); } + catch (InvalidDefinitionException ex) { + throw new HttpMessageNotReadableException("Could not read document: " + ex.getMessage(), ex, HttpStatus.INTERNAL_SERVER_ERROR); + } catch (IOException ex) { throw new HttpMessageNotReadableException("Could not read document: " + ex.getMessage(), ex); } diff --git a/spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java b/spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java index 7fcf96bb083..812190ce484 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/json/MappingJackson2HttpMessageConverterTests.java @@ -31,9 +31,11 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ser.FilterProvider; import com.fasterxml.jackson.databind.ser.impl.SimpleBeanPropertyFilter; import com.fasterxml.jackson.databind.ser.impl.SimpleFilterProvider; +import org.junit.Assert; import org.junit.Test; import org.springframework.core.ParameterizedTypeReference; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.MockHttpInputMessage; import org.springframework.http.MockHttpOutputMessage; @@ -380,6 +382,22 @@ public class MappingJackson2HttpMessageConverterTests { assertTrue(result.contains("\"number\":123")); } + @Test + public void readWithNoDefaultConstructor() throws Exception { + String body = "{\"property1\":\"foo\",\"property2\":\"bar\"}"; + MockHttpInputMessage inputMessage = new MockHttpInputMessage(body.getBytes("UTF-8")); + inputMessage.getHeaders().setContentType(new MediaType("application", "json")); + try { + converter.read(BeanWithNoDefaultConstructor.class, inputMessage); + } + catch (HttpMessageNotReadableException ex) { + assertTrue(ex.getErrorStatus().isPresent()); + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, ex.getErrorStatus().get()); + return; + } + fail(); + } + interface MyInterface { @@ -537,4 +555,25 @@ public class MappingJackson2HttpMessageConverterTests { } } + 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; + } + + } + } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java index 80e61a3881d..375c582268e 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java @@ -27,6 +27,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.ConversionNotSupportedException; import org.springframework.beans.TypeMismatchException; import org.springframework.core.Ordered; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.http.converter.HttpMessageNotWritableException; @@ -354,7 +355,7 @@ public class DefaultHandlerExceptionResolver extends AbstractHandlerExceptionRes if (logger.isWarnEnabled()) { logger.warn("Failed to read HTTP message: " + ex); } - response.sendError(HttpServletResponse.SC_BAD_REQUEST); + response.sendError(ex.getErrorStatus().orElse(HttpStatus.BAD_REQUEST).value()); return new ModelAndView(); }