From 6ab47db19d2089cbd78739de1c13e79a5bd2d264 Mon Sep 17 00:00:00 2001
From: Andy Wilkinson <awilkinson@pivotal.io>
Date: Tue, 18 Nov 2014 10:41:27 +0000
Subject: [PATCH] Omit null pathInfo from ErrorPageFilter's error message

Fixes gh-1944
---
 .../boot/context/web/ErrorPageFilter.java     | 15 ++++---
 .../context/web/ErrorPageFilterTests.java     | 40 +++++++++++++++++++
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java b/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java
index a4340ac752..cb29b3a333 100644
--- a/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java
+++ b/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java
@@ -164,9 +164,9 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple
 			IOException {
 
 		if (logger.isErrorEnabled()) {
-			String message = "Forwarding to error page from request ["
-					+ request.getServletPath() + request.getPathInfo()
-					+ "] due to exception [" + ex.getMessage() + "]";
+			String message = "Forwarding to error page from request "
+					+ getDescription(request) + " due to exception [" + ex.getMessage()
+					+ "]";
 			logger.error(message, ex);
 		}
 
@@ -179,9 +179,14 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple
 		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) {
-		String message = "Cannot forward to error page for request to "
-				+ request.getRequestURI() + " as the response has already been"
+		String message = "Cannot forward to error page for request "
+				+ getDescription(request) + " 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"
diff --git a/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java b/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java
index fe73f0b146..19dc76992d 100644
--- a/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java
+++ b/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java
@@ -25,14 +25,17 @@ import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpServletResponseWrapper;
 
+import org.junit.Rule;
 import org.junit.Test;
 import org.springframework.boot.context.embedded.ErrorPage;
+import org.springframework.boot.test.OutputCapture;
 import org.springframework.http.HttpStatus;
 import org.springframework.mock.web.MockFilterChain;
 import org.springframework.mock.web.MockFilterConfig;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
@@ -61,6 +64,9 @@ public class ErrorPageFilterTests {
 
 	private MockFilterChain chain = new MockFilterChain();
 
+	@Rule
+	public OutputCapture output = new OutputCapture();
+
 	@Test
 	public void notAnError() throws Exception {
 		this.filter.doFilter(this.request, this.response, this.chain);
@@ -359,4 +365,38 @@ public class ErrorPageFilterTests {
 		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]"));
+	}
+
 }