Skip to content

Commit 80767ff

Browse files
committed
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 spring-projects/spring-boot#3623 this commit rollbacks those changes and clarifies the behavior on the javadoc of the annotation itself. Issue: SPR-11193, SPR-13226
1 parent 1ff3af6 commit 80767ff

File tree

3 files changed

+17
-8
lines changed

3 files changed

+17
-8
lines changed

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,23 @@
3030
* {@link #reason} that should be returned.
3131
*
3232
* <p>The status code is applied to the HTTP response when the handler
33-
* method is invoked, or whenever said exception is thrown.
33+
* method is invoked.
34+
*
35+
* <p><strong>Note:</strong> when using this annotation on an exception class,
36+
* or when setting the {@code reason} attribute of the annotation,
37+
* the {@code HttpServletResponse.sendError} method will be used.
38+
*
39+
* With {@code HttpServletResponse.sendError}, the response is considered
40+
* complete and should not be written to any further.
41+
* Furthermore servlet container will typically write an HTML error page
42+
* therefore making the use of a reason unsuitable for REST APIs.
43+
* For such cases prefer the use of {@link org.springframework.http.ResponseEntity}
44+
* as a return type and avoid {@code ResponseStatus} altogether.
3445
*
3546
* @author Arjen Poutsma
3647
* @author Sam Brannen
3748
* @see org.springframework.web.servlet.mvc.annotation.ResponseStatusExceptionResolver
49+
* @see javax.servlet.http.HttpServletResponse#sendError(int, String)
3850
* @since 3.0
3951
*/
4052
@Target({ElementType.TYPE, ElementType.METHOD})
@@ -54,18 +66,13 @@
5466
* typically be changed to something more appropriate.
5567
* @since 4.2
5668
* @see javax.servlet.http.HttpServletResponse#setStatus(int)
69+
* @see javax.servlet.http.HttpServletResponse#sendError(int)
5770
*/
5871
@AliasFor("value")
5972
HttpStatus code() default HttpStatus.INTERNAL_SERVER_ERROR;
6073

6174
/**
6275
* The <em>reason</em> to be used for the response.
63-
* <p><strong>Note:</strong> due to the use of
64-
* {@code HttpServletResponse.sendError(int, String)}, the response will be
65-
* considered complete and should not be written to any further. Furthermore
66-
* servlet container will typically write an HTML error page therefore making
67-
* the use of a reason unsuitable for REST APIs. For such cases prefer
68-
* sending error details in the body of the response.
6976
* @see javax.servlet.http.HttpServletResponse#sendError(int, String)
7077
*/
7178
String reason() default "";

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ protected ModelAndView resolveResponseStatus(ResponseStatus responseStatus, Http
100100
reason = this.messageSource.getMessage(reason, null, reason, LocaleContextHolder.getLocale());
101101
}
102102
if (!StringUtils.hasLength(reason)) {
103-
response.setStatus(statusCode);
103+
response.sendError(statusCode);
104104
}
105105
else {
106106
response.sendError(statusCode, reason);

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ public void statusCode() {
5757
assertNotNull("No ModelAndView returned", mav);
5858
assertTrue("No Empty ModelAndView returned", mav.isEmpty());
5959
assertEquals("Invalid status code", 400, response.getStatus());
60+
assertTrue("Response has not been committed", response.isCommitted());
6061
}
6162

6263
@Test
@@ -67,6 +68,7 @@ public void statusCodeAndReason() {
6768
assertTrue("No Empty ModelAndView returned", mav.isEmpty());
6869
assertEquals("Invalid status code", 410, response.getStatus());
6970
assertEquals("Invalid status reason", "You suck!", response.getErrorMessage());
71+
assertTrue("Response has not been committed", response.isCommitted());
7072
}
7173

7274
@Test

0 commit comments

Comments
 (0)