Merge branch '1.1.x'

This commit is contained in:
Andy Wilkinson
2014-09-25 19:06:37 +01:00
3 changed files with 121 additions and 30 deletions

View File

@@ -52,6 +52,7 @@ import org.springframework.web.filter.OncePerRequestFilter;
*
* @author Dave Syer
* @author Phillip Webb
* @author Andy Wilkinson
*/
@Component
@Order(Ordered.HIGHEST_PRECEDENCE)
@@ -124,6 +125,12 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple
private void handleErrorStatus(HttpServletRequest request,
HttpServletResponse response, int status, String message)
throws ServletException, IOException {
if (response.isCommitted()) {
handleCommittedResponse(request, null);
return;
}
String errorPath = getErrorPath(this.statuses, status);
if (errorPath == null) {
response.sendError(status, message);
@@ -132,6 +139,7 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple
response.setStatus(status);
setErrorAttributes(request, status, message);
request.getRequestDispatcher(errorPath).forward(request, response);
}
private void handleException(HttpServletRequest request,
@@ -143,35 +151,51 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple
rethrow(ex);
return;
}
if (logger.isErrorEnabled()) {
String message = "Forwarding to error page from request ["
+ request.getServletPath() + request.getPathInfo()
+ "] due to exception [" + ex.getMessage() + "]";
logger.error(message, ex);
if (response.isCommitted()) {
handleCommittedResponse(request, ex);
return;
}
setErrorAttributes(request, 500, ex.getMessage());
request.setAttribute(ERROR_EXCEPTION, ex);
request.setAttribute(ERROR_EXCEPTION_TYPE, type.getName());
forwardToErrorPage(errorPath, request, wrapped, ex);
}
private void forwardToErrorPage(String path, HttpServletRequest request,
HttpServletResponse response, Throwable ex) throws ServletException,
IOException {
if (response.isCommitted()) {
String message = "Cannot forward to error page for" + request.getRequestURI()
+ " (response is committed), so this response may have "
+ "the wrong status code";
// User might see the error page without all the data here but throwing the
// exception isn't going to help anyone (we'll log it to be on the safe side)
if (logger.isErrorEnabled()) {
String message = "Forwarding to error page from request ["
+ request.getServletPath() + request.getPathInfo()
+ "] due to exception [" + ex.getMessage() + "]";
logger.error(message, ex);
return;
}
setErrorAttributes(request, 500, ex.getMessage());
request.setAttribute(ERROR_EXCEPTION, ex);
request.setAttribute(ERROR_EXCEPTION_TYPE, ex.getClass().getName());
response.reset();
response.sendError(500, ex.getMessage());
request.getRequestDispatcher(path).forward(request, response);
}
private void handleCommittedResponse(HttpServletRequest request, Throwable ex) {
String message = "Cannot forward to error page for request to "
+ request.getRequestURI() + " as the response has already been"
+ " committed. As a result, the response may have the wrong status"
+ " code. If your application is running on WebSphere Application"
+ " Server you may be able to resolve this problem by setting "
+ " com.ibm.ws.webcontainer.invokeFlushAfterService to false";
if (ex == null) {
logger.error(message);
}
else {
// User might see the error page without all the data here but throwing the
// exception isn't going to help anyone (we'll log it to be on the safe side)
logger.error(message, ex);
}
}
private String getErrorPath(Map<Integer, String> map, Integer status) {
if (map.containsKey(status)) {
return map.get(status);
@@ -243,6 +267,8 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple
private String message;
private boolean errorToSend;
public ErrorWrapperResponse(HttpServletResponse response) {
super(response);
this.status = response.getStatus();
@@ -257,6 +283,8 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple
public void sendError(int status, String message) throws IOException {
this.status = status;
this.message = message;
this.errorToSend = true;
}
@Override
@@ -264,6 +292,15 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple
return this.status;
}
@Override
public void flushBuffer() throws IOException {
if (this.errorToSend && !isCommitted()) {
((HttpServletResponse) getResponse())
.sendError(this.status, this.message);
}
super.flushBuffer();
}
public String getMessage() {
return this.message;
}

View File

@@ -34,6 +34,8 @@ import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
@@ -43,6 +45,7 @@ import static org.junit.Assert.assertTrue;
* Tests for {@link ErrorPageFilter}.
*
* @author Dave Syer
* @author Andy Wilkinson
*/
public class ErrorPageFilterTests {
@@ -61,6 +64,7 @@ public class ErrorPageFilterTests {
assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getResponse(),
equalTo((ServletResponse) this.response));
assertTrue(this.response.isCommitted());
assertThat(this.response.getForwardedUrl(), is(nullValue()));
}
@Test
@@ -83,6 +87,7 @@ public class ErrorPageFilterTests {
assertThat(wrapper.getStatus(), equalTo(401));
// The real response has to be 401 as well...
assertThat(this.response.getStatus(), equalTo(401));
assertThat(this.response.getForwardedUrl(), equalTo("/error"));
}
@Test
@@ -103,11 +108,12 @@ public class ErrorPageFilterTests {
equalTo((ServletResponse) this.response));
assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getStatus(),
equalTo(400));
assertThat(this.response.getForwardedUrl(), is(nullValue()));
assertTrue(this.response.isCommitted());
}
@Test
public void responseUncommitted() throws Exception {
public void responseUncommittedWithoutErrorPage() throws Exception {
this.chain = new MockFilterChain() {
@Override
public void doFilter(ServletRequest request, ServletResponse response)
@@ -122,6 +128,7 @@ public class ErrorPageFilterTests {
equalTo((ServletResponse) this.response));
assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getStatus(),
equalTo(400));
assertThat(this.response.getForwardedUrl(), is(nullValue()));
assertTrue(this.response.isCommitted());
}
@@ -159,6 +166,7 @@ public class ErrorPageFilterTests {
assertThat(this.request.getAttribute(RequestDispatcher.ERROR_MESSAGE),
equalTo((Object) "BAD"));
assertTrue(this.response.isCommitted());
assertThat(this.response.getForwardedUrl(), equalTo("/error"));
}
@Test
@@ -180,6 +188,26 @@ public class ErrorPageFilterTests {
assertThat(this.request.getAttribute(RequestDispatcher.ERROR_MESSAGE),
equalTo((Object) "BAD"));
assertTrue(this.response.isCommitted());
assertThat(this.response.getForwardedUrl(), equalTo("/400"));
}
@Test
public void statusErrorWithCommittedResponse() throws Exception {
this.filter.addErrorPages(new ErrorPage(HttpStatus.BAD_REQUEST, "/400"));
this.chain = new MockFilterChain() {
@Override
public void doFilter(ServletRequest request, ServletResponse response)
throws IOException, ServletException {
((HttpServletResponse) response).sendError(400, "BAD");
response.flushBuffer();
super.doFilter(request, response);
}
};
this.filter.doFilter(this.request, this.response, this.chain);
assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getStatus(),
equalTo(400));
assertTrue(this.response.isCommitted());
assertThat(this.response.getForwardedUrl(), is(nullValue()));
}
@Test
@@ -203,6 +231,23 @@ public class ErrorPageFilterTests {
assertThat(this.request.getAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE),
equalTo((Object) RuntimeException.class.getName()));
assertTrue(this.response.isCommitted());
assertThat(this.response.getForwardedUrl(), equalTo("/500"));
}
@Test
public void exceptionErrorWithCommittedResponse() throws Exception {
this.filter.addErrorPages(new ErrorPage(RuntimeException.class, "/500"));
this.chain = new MockFilterChain() {
@Override
public void doFilter(ServletRequest request, ServletResponse response)
throws IOException, ServletException {
super.doFilter(request, response);
response.flushBuffer();
throw new RuntimeException("BAD");
}
};
this.filter.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getForwardedUrl(), is(nullValue()));
}
@Test