From c5e360d886931f761c2be5ff4b31d5df2042d415 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 17 Oct 2014 13:03:03 -0400 Subject: [PATCH] Fix regression with raw ResponseEntity type This fix addresses a 4.1.1 regression where a raw ResponseEntity return value (used to return potentially a different kind of body) caused an exception. The regression came from the fact we now try to render a null body in order to give ResponseBodyAdvice a chance to substitute a different value. That in turn means we have to try to determine the body type from the method signature. This change improves the logic for extracting the generic parameter type to accommodate a raw ResponseEntity class. Also we avoid raising HttpMediaTypeNotAcceptableException if the value to be rendered is null. Issue: SPR-12287 --- ...stractMessageConverterMethodProcessor.java | 14 ++++---- .../annotation/HttpEntityMethodProcessor.java | 18 +++++++---- .../RequestResponseBodyMethodProcessor.java | 6 ++-- .../ServletInvocableHandlerMethodTests.java | 32 +++++++++++++++---- 4 files changed, 49 insertions(+), 21 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java index eba7ce50f4a..d912e1cc84a 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java @@ -117,10 +117,6 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe throws IOException, HttpMediaTypeNotAcceptableException { Class returnValueClass = getReturnValueType(returnValue, returnType); - if (returnValue == null && Void.class.equals(returnValueClass)) { - return; - } - HttpServletRequest servletRequest = inputMessage.getServletRequest(); List requestedMediaTypes = getAcceptableMediaTypes(servletRequest); List producibleMediaTypes = getProducibleMediaTypes(servletRequest, returnValueClass); @@ -134,7 +130,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } } if (compatibleMediaTypes.isEmpty()) { - throw new HttpMediaTypeNotAcceptableException(producibleMediaTypes); + if (returnValue != null) { + throw new HttpMediaTypeNotAcceptableException(producibleMediaTypes); + } + return; } List mediaTypes = new ArrayList(compatibleMediaTypes); @@ -169,7 +168,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } } } - throw new HttpMediaTypeNotAcceptableException(this.allSupportedMediaTypes); + + if (returnValue != null) { + throw new HttpMediaTypeNotAcceptableException(this.allSupportedMediaTypes); + } } /** diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java index 689ee869d60..27989f45a36 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java @@ -102,12 +102,17 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro Type parameterType = parameter.getGenericParameterType(); if (parameterType instanceof ParameterizedType) { ParameterizedType type = (ParameterizedType) parameterType; - if (type.getActualTypeArguments().length == 1) { - return type.getActualTypeArguments()[0]; + if (type.getActualTypeArguments().length != 1) { + throw new IllegalArgumentException("Expected single generic parameter on '" + + parameter.getParameterName() + "' in method " + parameter.getMethod()); } + return type.getActualTypeArguments()[0]; + } + else if (parameterType instanceof Class) { + return Object.class; } throw new IllegalArgumentException("HttpEntity parameter '" + parameter.getParameterName() + - "' in method " + parameter.getMethod() + " is not parameterized or has more than one parameter"); + "' in method " + parameter.getMethod() + " is not parameterized"); } @Override @@ -134,9 +139,10 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro } Object body = responseEntity.getBody(); - if (body != null || getAdviceChain().hasAdvice()) { - writeWithMessageConverters(body, returnType, inputMessage, outputMessage); - } + + // Try even with null body. ResponseBodyAdvice could get involved. + writeWithMessageConverters(body, returnType, inputMessage, outputMessage); + // Ensure headers are flushed even if no body was written outputMessage.getBody(); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java index 3d11a972313..08fe6a9e968 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java @@ -194,9 +194,9 @@ public class RequestResponseBodyMethodProcessor extends AbstractMessageConverter throws IOException, HttpMediaTypeNotAcceptableException { mavContainer.setRequestHandled(true); - if (returnValue != null || getAdviceChain().hasAdvice()) { - writeWithMessageConverters(returnValue, returnType, webRequest); - } + + // Try even with null return value. ResponseBodyAdvice could get involved. + writeWithMessageConverters(returnValue, returnType, webRequest); } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java index 6413db4a52a..e064a81a9df 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.java @@ -180,7 +180,7 @@ public class ServletInvocableHandlerMethodTests { List> converters = new ArrayList>(); converters.add(new StringHttpMessageConverter()); this.returnValueHandlers.addHandler(new HttpEntityMethodProcessor(converters)); - ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handle"); + ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handleDeferred"); handlerMethod = handlerMethod.wrapConcurrentResult(new ResponseEntity<>("bar", HttpStatus.OK)); handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); @@ -196,7 +196,7 @@ public class ServletInvocableHandlerMethodTests { List advice = Arrays.asList(mock(ResponseBodyAdvice.class)); HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice); this.returnValueHandlers.addHandler(processor); - ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handle"); + ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handleDeferred"); handlerMethod = handlerMethod.wrapConcurrentResult(new ResponseEntity<>(HttpStatus.OK)); handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); @@ -211,7 +211,7 @@ public class ServletInvocableHandlerMethodTests { List advice = Arrays.asList(mock(ResponseBodyAdvice.class)); HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice); this.returnValueHandlers.addHandler(processor); - ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handle"); + ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handleDeferred"); handlerMethod = handlerMethod.wrapConcurrentResult(null); handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); @@ -219,13 +219,28 @@ public class ServletInvocableHandlerMethodTests { assertEquals("", this.response.getContentAsString()); } + // SPR-12287 (16/Oct/14 comments) + + @Test + public void responseEntityRawTypeWithNullBody() throws Exception { + List> converters = Arrays.asList(new StringHttpMessageConverter()); + List advice = Arrays.asList(mock(ResponseBodyAdvice.class)); + HttpEntityMethodProcessor processor = new HttpEntityMethodProcessor(converters, null, advice); + this.returnValueHandlers.addHandler(processor); + ServletInvocableHandlerMethod handlerMethod = getHandlerMethod(new ResponseEntityHandler(), "handleRawType"); + handlerMethod.invokeAndHandle(this.webRequest, this.mavContainer); + + assertEquals(200, this.response.getStatus()); + assertEquals("", this.response.getContentAsString()); + } + private ServletInvocableHandlerMethod getHandlerMethod(Object controller, String methodName, Class... argTypes) throws NoSuchMethodException { Method method = controller.getClass().getDeclaredMethod(methodName, argTypes); ServletInvocableHandlerMethod handlerMethod = new ServletInvocableHandlerMethod(controller, method); - handlerMethod.setHandlerMethodArgumentResolvers(argumentResolvers); - handlerMethod.setHandlerMethodReturnValueHandlers(returnValueHandlers); + handlerMethod.setHandlerMethodArgumentResolvers(this.argumentResolvers); + handlerMethod.setHandlerMethodReturnValueHandlers(this.returnValueHandlers); return handlerMethod; } @@ -277,9 +292,14 @@ public class ServletInvocableHandlerMethodTests { private static class ResponseEntityHandler { @SuppressWarnings("unused") - public DeferredResult> handle() { + public DeferredResult> handleDeferred() { return new DeferredResult<>(); } + + @SuppressWarnings("unused") + public ResponseEntity handleRawType() { + return ResponseEntity.ok().build(); + } } private static class ExceptionRaisingReturnValueHandler implements HandlerMethodReturnValueHandler {