Browse Source

Use sendError in ResponseStatusExceptionResolver

Prior to this commit, the `ResponseStatusExceptionResolver` would use:
* `HttpServletResponse.sendError` if both a status and a reason are set
on the `@ResponseStatus` annotation
* `HttpServletResponse.setStatus` if only a status is set on the
`@ResponseStatus` annotation

This is actually a change of behavior, since this Resolver was using
`sendError` in all cases previously.

Because this change can create issues such as
https://github.com/spring-projects/spring-boot/issues/3623
this commit rollbacks those changes and clarifies the behavior on the
javadoc of the annotation itself.

Issue: SPR-11193, SPR-13226
pull/851/head
Brian Clozel 11 years ago
parent
commit
80767ff6e9
  1. 21
      spring-web/src/main/java/org/springframework/web/bind/annotation/ResponseStatus.java
  2. 2
      spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/annotation/ResponseStatusExceptionResolver.java
  3. 2
      spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/ResponseStatusExceptionResolverTests.java

21
spring-web/src/main/java/org/springframework/web/bind/annotation/ResponseStatus.java

@ -30,11 +30,23 @@ import org.springframework.http.HttpStatus; @@ -30,11 +30,23 @@ import org.springframework.http.HttpStatus;
* {@link #reason} that should be returned.
*
* <p>The status code is applied to the HTTP response when the handler
* method is invoked, or whenever said exception is thrown.
* method is invoked.
*
* <p><strong>Note:</strong> when using this annotation on an exception class,
* or when setting the {@code reason} attribute of the annotation,
* the {@code HttpServletResponse.sendError} method will be used.
*
* With {@code HttpServletResponse.sendError}, the response is considered
* complete and should not be written to any further.
* Furthermore servlet container will typically write an HTML error page
* therefore making the use of a reason unsuitable for REST APIs.
* For such cases prefer the use of {@link org.springframework.http.ResponseEntity}
* as a return type and avoid {@code ResponseStatus} altogether.
*
* @author Arjen Poutsma
* @author Sam Brannen
* @see org.springframework.web.servlet.mvc.annotation.ResponseStatusExceptionResolver
* @see javax.servlet.http.HttpServletResponse#sendError(int, String)
* @since 3.0
*/
@Target({ElementType.TYPE, ElementType.METHOD})
@ -54,18 +66,13 @@ public @interface ResponseStatus { @@ -54,18 +66,13 @@ public @interface ResponseStatus {
* typically be changed to something more appropriate.
* @since 4.2
* @see javax.servlet.http.HttpServletResponse#setStatus(int)
* @see javax.servlet.http.HttpServletResponse#sendError(int)
*/
@AliasFor("value")
HttpStatus code() default HttpStatus.INTERNAL_SERVER_ERROR;
/**
* The <em>reason</em> to be used for the response.
* <p><strong>Note:</strong> due to the use of
* {@code HttpServletResponse.sendError(int, String)}, the response will be
* considered complete and should not be written to any further. Furthermore
* servlet container will typically write an HTML error page therefore making
* the use of a reason unsuitable for REST APIs. For such cases prefer
* sending error details in the body of the response.
* @see javax.servlet.http.HttpServletResponse#sendError(int, String)
*/
String reason() default "";

2
spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/annotation/ResponseStatusExceptionResolver.java

@ -100,7 +100,7 @@ public class ResponseStatusExceptionResolver extends AbstractHandlerExceptionRes @@ -100,7 +100,7 @@ public class ResponseStatusExceptionResolver extends AbstractHandlerExceptionRes
reason = this.messageSource.getMessage(reason, null, reason, LocaleContextHolder.getLocale());
}
if (!StringUtils.hasLength(reason)) {
response.setStatus(statusCode);
response.sendError(statusCode);
}
else {
response.sendError(statusCode, reason);

2
spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/ResponseStatusExceptionResolverTests.java

@ -57,6 +57,7 @@ public class ResponseStatusExceptionResolverTests { @@ -57,6 +57,7 @@ public class ResponseStatusExceptionResolverTests {
assertNotNull("No ModelAndView returned", mav);
assertTrue("No Empty ModelAndView returned", mav.isEmpty());
assertEquals("Invalid status code", 400, response.getStatus());
assertTrue("Response has not been committed", response.isCommitted());
}
@Test
@ -67,6 +68,7 @@ public class ResponseStatusExceptionResolverTests { @@ -67,6 +68,7 @@ public class ResponseStatusExceptionResolverTests {
assertTrue("No Empty ModelAndView returned", mav.isEmpty());
assertEquals("Invalid status code", 410, response.getStatus());
assertEquals("Invalid status reason", "You suck!", response.getErrorMessage());
assertTrue("Response has not been committed", response.isCommitted());
}
@Test

Loading…
Cancel
Save