Omit null pathInfo from ErrorPageFilter's error message
Fixes gh-1944
This commit is contained in:
parent
41cb567894
commit
6ab47db19d
|
@ -164,9 +164,9 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple
|
||||||
IOException {
|
IOException {
|
||||||
|
|
||||||
if (logger.isErrorEnabled()) {
|
if (logger.isErrorEnabled()) {
|
||||||
String message = "Forwarding to error page from request ["
|
String message = "Forwarding to error page from request "
|
||||||
+ request.getServletPath() + request.getPathInfo()
|
+ getDescription(request) + " due to exception [" + ex.getMessage()
|
||||||
+ "] due to exception [" + ex.getMessage() + "]";
|
+ "]";
|
||||||
logger.error(message, ex);
|
logger.error(message, ex);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -179,9 +179,14 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple
|
||||||
request.getRequestDispatcher(path).forward(request, response);
|
request.getRequestDispatcher(path).forward(request, response);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private String getDescription(HttpServletRequest request) {
|
||||||
|
return "[" + request.getServletPath()
|
||||||
|
+ (request.getPathInfo() == null ? "" : request.getPathInfo()) + "]";
|
||||||
|
}
|
||||||
|
|
||||||
private void handleCommittedResponse(HttpServletRequest request, Throwable ex) {
|
private void handleCommittedResponse(HttpServletRequest request, Throwable ex) {
|
||||||
String message = "Cannot forward to error page for request to "
|
String message = "Cannot forward to error page for request "
|
||||||
+ request.getRequestURI() + " as the response has already been"
|
+ getDescription(request) + " as the response has already been"
|
||||||
+ " committed. As a result, the response may have the wrong status"
|
+ " committed. As a result, the response may have the wrong status"
|
||||||
+ " code. If your application is running on WebSphere Application"
|
+ " code. If your application is running on WebSphere Application"
|
||||||
+ " Server you may be able to resolve this problem by setting"
|
+ " Server you may be able to resolve this problem by setting"
|
||||||
|
|
|
@ -25,14 +25,17 @@ import javax.servlet.ServletResponse;
|
||||||
import javax.servlet.http.HttpServletResponse;
|
import javax.servlet.http.HttpServletResponse;
|
||||||
import javax.servlet.http.HttpServletResponseWrapper;
|
import javax.servlet.http.HttpServletResponseWrapper;
|
||||||
|
|
||||||
|
import org.junit.Rule;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
import org.springframework.boot.context.embedded.ErrorPage;
|
import org.springframework.boot.context.embedded.ErrorPage;
|
||||||
|
import org.springframework.boot.test.OutputCapture;
|
||||||
import org.springframework.http.HttpStatus;
|
import org.springframework.http.HttpStatus;
|
||||||
import org.springframework.mock.web.MockFilterChain;
|
import org.springframework.mock.web.MockFilterChain;
|
||||||
import org.springframework.mock.web.MockFilterConfig;
|
import org.springframework.mock.web.MockFilterConfig;
|
||||||
import org.springframework.mock.web.MockHttpServletRequest;
|
import org.springframework.mock.web.MockHttpServletRequest;
|
||||||
import org.springframework.mock.web.MockHttpServletResponse;
|
import org.springframework.mock.web.MockHttpServletResponse;
|
||||||
|
|
||||||
|
import static org.hamcrest.Matchers.containsString;
|
||||||
import static org.hamcrest.Matchers.equalTo;
|
import static org.hamcrest.Matchers.equalTo;
|
||||||
import static org.hamcrest.Matchers.is;
|
import static org.hamcrest.Matchers.is;
|
||||||
import static org.hamcrest.Matchers.nullValue;
|
import static org.hamcrest.Matchers.nullValue;
|
||||||
|
@ -61,6 +64,9 @@ public class ErrorPageFilterTests {
|
||||||
|
|
||||||
private MockFilterChain chain = new MockFilterChain();
|
private MockFilterChain chain = new MockFilterChain();
|
||||||
|
|
||||||
|
@Rule
|
||||||
|
public OutputCapture output = new OutputCapture();
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void notAnError() throws Exception {
|
public void notAnError() throws Exception {
|
||||||
this.filter.doFilter(this.request, this.response, this.chain);
|
this.filter.doFilter(this.request, this.response, this.chain);
|
||||||
|
@ -359,4 +365,38 @@ public class ErrorPageFilterTests {
|
||||||
verify(committedResponse, times(0)).flushBuffer();
|
verify(committedResponse, times(0)).flushBuffer();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void errorMessageForRequestWithoutPathInfo() throws IOException,
|
||||||
|
ServletException {
|
||||||
|
this.request.setServletPath("/test");
|
||||||
|
this.filter.addErrorPages(new ErrorPage("/error"));
|
||||||
|
this.chain = new MockFilterChain() {
|
||||||
|
@Override
|
||||||
|
public void doFilter(ServletRequest request, ServletResponse response)
|
||||||
|
throws IOException, ServletException {
|
||||||
|
super.doFilter(request, response);
|
||||||
|
throw new RuntimeException();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
this.filter.doFilter(this.request, this.response, this.chain);
|
||||||
|
assertThat(this.output.toString(), containsString("request [/test]"));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void errorMessageForRequestWithPathInfo() throws IOException, ServletException {
|
||||||
|
this.request.setServletPath("/test");
|
||||||
|
this.request.setPathInfo("/alpha");
|
||||||
|
this.filter.addErrorPages(new ErrorPage("/error"));
|
||||||
|
this.chain = new MockFilterChain() {
|
||||||
|
@Override
|
||||||
|
public void doFilter(ServletRequest request, ServletResponse response)
|
||||||
|
throws IOException, ServletException {
|
||||||
|
super.doFilter(request, response);
|
||||||
|
throw new RuntimeException();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
this.filter.doFilter(this.request, this.response, this.chain);
|
||||||
|
assertThat(this.output.toString(), containsString("request [/test/alpha]"));
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue