Fix logic error in ErrorPageFilter (fixes gh-1149)
This commit is contained in:
parent
91bbd20ca0
commit
b0bf9c776f
|
@ -36,4 +36,9 @@ public class WelcomeController {
|
||||||
return "welcome";
|
return "welcome";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@RequestMapping("/foo")
|
||||||
|
public String foo(Map<String, Object> model) {
|
||||||
|
throw new RuntimeException("Foo");
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -135,13 +135,13 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple
|
||||||
setErrorAttributes(request, 500, ex.getMessage());
|
setErrorAttributes(request, 500, ex.getMessage());
|
||||||
request.setAttribute(ERROR_EXCEPTION, ex);
|
request.setAttribute(ERROR_EXCEPTION, ex);
|
||||||
request.setAttribute(ERROR_EXCEPTION_TYPE, type.getName());
|
request.setAttribute(ERROR_EXCEPTION_TYPE, type.getName());
|
||||||
wrapped.sendError(500, ex.getMessage());
|
|
||||||
forwardToErrorPage(errorPath, request, wrapped, ex);
|
forwardToErrorPage(errorPath, request, wrapped, ex);
|
||||||
}
|
}
|
||||||
|
|
||||||
private void forwardToErrorPage(String path, HttpServletRequest request,
|
private void forwardToErrorPage(String path, HttpServletRequest request,
|
||||||
ServletResponse response, Throwable ex) throws ServletException, IOException {
|
HttpServletResponse response, Throwable ex) throws ServletException,
|
||||||
if (!response.isCommitted()) {
|
IOException {
|
||||||
|
if (response.isCommitted()) {
|
||||||
String message = "Cannot forward to error page for" + request.getRequestURI()
|
String message = "Cannot forward to error page for" + request.getRequestURI()
|
||||||
+ " (response is committed), so this response may have "
|
+ " (response is committed), so this response may have "
|
||||||
+ "the wrong status code";
|
+ "the wrong status code";
|
||||||
|
@ -151,6 +151,7 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
response.reset();
|
response.reset();
|
||||||
|
response.sendError(500, ex.getMessage());
|
||||||
request.getRequestDispatcher(path).forward(request, response);
|
request.getRequestDispatcher(path).forward(request, response);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -59,6 +59,44 @@ public class ErrorPageFilterTests {
|
||||||
equalTo((ServletResponse) this.response));
|
equalTo((ServletResponse) this.response));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void responseCommitted() throws Exception {
|
||||||
|
this.filter.addErrorPages(new ErrorPage("/error"));
|
||||||
|
this.response.setCommitted(true);
|
||||||
|
this.chain = new MockFilterChain() {
|
||||||
|
@Override
|
||||||
|
public void doFilter(ServletRequest request, ServletResponse response)
|
||||||
|
throws IOException, ServletException {
|
||||||
|
((HttpServletResponse) response).sendError(400, "BAD");
|
||||||
|
super.doFilter(request, response);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
this.filter.doFilter(this.request, this.response, this.chain);
|
||||||
|
assertThat(this.chain.getRequest(), equalTo((ServletRequest) this.request));
|
||||||
|
assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getResponse(),
|
||||||
|
equalTo((ServletResponse) this.response));
|
||||||
|
assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getStatus(),
|
||||||
|
equalTo(400));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void responseUncommitted() throws Exception {
|
||||||
|
this.chain = new MockFilterChain() {
|
||||||
|
@Override
|
||||||
|
public void doFilter(ServletRequest request, ServletResponse response)
|
||||||
|
throws IOException, ServletException {
|
||||||
|
((HttpServletResponse) response).sendError(400, "BAD");
|
||||||
|
super.doFilter(request, response);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
this.filter.doFilter(this.request, this.response, this.chain);
|
||||||
|
assertThat(this.chain.getRequest(), equalTo((ServletRequest) this.request));
|
||||||
|
assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getResponse(),
|
||||||
|
equalTo((ServletResponse) this.response));
|
||||||
|
assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getStatus(),
|
||||||
|
equalTo(400));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void globalError() throws Exception {
|
public void globalError() throws Exception {
|
||||||
this.filter.addErrorPages(new ErrorPage("/error"));
|
this.filter.addErrorPages(new ErrorPage("/error"));
|
||||||
|
|
Loading…
Reference in New Issue