From d52fc3bd2fb96de0f92e51782523ecd2b06646b0 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 11 May 2012 17:41:14 -0400 Subject: [PATCH] Prevent response updates if @ResponseStatus has reason When @ResponseStatus has a reason and servletResponse.sendError() is called, the response is committed and should no longer be written to. After this change, the ServletInvocableHandlerMethod will mark the response fully handled and will ignore any non-null return values. Issue: SPR-9159 --- .../web/bind/annotation/ResponseStatus.java | 5 +-- .../ServletInvocableHandlerMethod.java | 4 +++ .../ServletInvocableHandlerMethodTests.java | 31 ++++++++++++++----- src/dist/changelog.txt | 3 +- 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/ResponseStatus.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/ResponseStatus.java index d07c65d8a86..e4df7821c19 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/ResponseStatus.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/ResponseStatus.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -46,7 +46,8 @@ public @interface ResponseStatus { /** * The reason to be used for the response.

If this element is not set, it will default to the standard status - * message for the status code. + * message for the status code. Note that due to the use of {@code HttpServletResponse.sendError(int, String)}, + * the response will be considered complete and should not be written to any further. * * @see javax.servlet.http.HttpServletResponse#sendError(int, String) */ diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java index 69d672acf2d..32d4920750f 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java @@ -104,6 +104,10 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod { return; } } + else if (StringUtils.hasText(this.responseReason)) { + mavContainer.setRequestHandled(true); + return; + } mavContainer.setRequestHandled(false); 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 2e1e62b0b2e..f92e8478fcc 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 @@ -72,19 +72,17 @@ public class ServletInvocableHandlerMethodTests { } @Test - public void nullReturnValueResponseStatus() throws Exception { + public void invokeAndHandle_VoidWithResponseStatus() throws Exception { ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("responseStatus"); handlerMethod.invokeAndHandle(webRequest, mavContainer); assertTrue("Null return value + @ResponseStatus should result in 'request handled'", mavContainer.isRequestHandled()); - assertEquals(HttpStatus.BAD_REQUEST.value(), response.getStatus()); - assertEquals("400 Bad Request", response.getErrorMessage()); } @Test - public void nullReturnValueHttpServletResponseArg() throws Exception { + public void invokeAndHandle_VoidWithHttpServletResponseArgument() throws Exception { argumentResolvers.addResolver(new ServletResponseMethodArgumentResolver()); ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("httpServletResponse", HttpServletResponse.class); @@ -95,7 +93,7 @@ public class ServletInvocableHandlerMethodTests { } @Test - public void nullReturnValueRequestNotModified() throws Exception { + public void invokeAndHandle_VoidRequestNotModified() throws Exception { webRequest.getNativeRequest(MockHttpServletRequest.class).addHeader("If-Modified-Since", 10 * 1000 * 1000); int lastModifiedTimestamp = 1000 * 1000; webRequest.checkNotModified(lastModifiedTimestamp); @@ -107,8 +105,20 @@ public class ServletInvocableHandlerMethodTests { mavContainer.isRequestHandled()); } + // SPR-9159 + + @Test + public void invokeAndHandle_NotVoidWithResponseStatusAndReason() throws Exception { + ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("responseStatusWithReason"); + handlerMethod.invokeAndHandle(webRequest, mavContainer); + + assertTrue("When a phrase is used, the response should not be used any more", mavContainer.isRequestHandled()); + assertEquals(HttpStatus.BAD_REQUEST.value(), response.getStatus()); + assertEquals("400 Bad Request", response.getErrorMessage()); + } + @Test(expected=HttpMessageNotWritableException.class) - public void exceptionWhileHandlingReturnValue() throws Exception { + public void invokeAndHandle_Exception() throws Exception { returnValueHandlers.addHandler(new ExceptionRaisingReturnValueHandler()); ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("handle"); @@ -117,7 +127,7 @@ public class ServletInvocableHandlerMethodTests { } @Test - public void dynamicReturnValue() throws Exception { + public void invokeAndHandle_DynamicReturnValue() throws Exception { argumentResolvers.addResolver(new RequestParamMethodArgumentResolver(null, false)); returnValueHandlers.addHandler(new ViewMethodReturnValueHandler()); returnValueHandlers.addHandler(new ViewNameMethodReturnValueHandler()); @@ -153,10 +163,15 @@ public class ServletInvocableHandlerMethodTests { return "view"; } - @ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "400 Bad Request") + @ResponseStatus(value = HttpStatus.BAD_REQUEST) public void responseStatus() { } + @ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "400 Bad Request") + public String responseStatusWithReason() { + return "foo"; + } + public void httpServletResponse(HttpServletResponse response) { } diff --git a/src/dist/changelog.txt b/src/dist/changelog.txt index f76e2acbbd6..7695816b797 100644 --- a/src/dist/changelog.txt +++ b/src/dist/changelog.txt @@ -14,12 +14,13 @@ Changes in version 3.2 M1 * fix case-sensitivity issue with some containers on access to 'Content-Disposition' header * add Servlet 3.0 based async support * fix issue with encoded params in UriComponentsBuilder -* add Jackson 2 HttpMessageConverter and View types +* add HttpMessageConverter and View types compatible with Jackson 2.0 * add pretty print option to Jackson HttpMessageConverter and View types * fix issue with resolving Errors controller method argument * detect controller methods via InitializingBean in RequestMappingHandlerMapping * translate IOException from Jackson to HttpMessageNotReadableException * fix content negotiation issue when sorting selected media types by quality value +* Prevent further writing to the response when @ResponseStatus contains a reason Changes in version 3.1.1 (2012-02-16)