diff --git a/spring-web/src/main/java/org/springframework/web/filter/AbstractRequestLoggingFilter.java b/spring-web/src/main/java/org/springframework/web/filter/AbstractRequestLoggingFilter.java index 33bb39754d8b..28e41c96e1a1 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/AbstractRequestLoggingFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/AbstractRequestLoggingFilter.java @@ -322,7 +322,8 @@ private String getAfterMessage(HttpServletRequest request) { protected String createMessage(HttpServletRequest request, String prefix, String suffix) { StringBuilder msg = new StringBuilder(); msg.append(prefix); - msg.append("uri=").append(request.getRequestURI()); + msg.append(request.getMethod()).append(" "); + msg.append(request.getRequestURI()); if (isIncludeQueryString()) { String queryString = request.getQueryString(); @@ -334,15 +335,15 @@ protected String createMessage(HttpServletRequest request, String prefix, String if (isIncludeClientInfo()) { String client = request.getRemoteAddr(); if (StringUtils.hasLength(client)) { - msg.append(";client=").append(client); + msg.append(", client=").append(client); } HttpSession session = request.getSession(false); if (session != null) { - msg.append(";session=").append(session.getId()); + msg.append(", session=").append(session.getId()); } String user = request.getRemoteUser(); if (user != null) { - msg.append(";user=").append(user); + msg.append(", user=").append(user); } } @@ -357,13 +358,13 @@ protected String createMessage(HttpServletRequest request, String prefix, String } } } - msg.append(";headers=").append(headers); + msg.append(", headers=").append(headers); } if (isIncludePayload()) { String payload = getMessagePayload(request); if (payload != null) { - msg.append(";payload=").append(payload); + msg.append(", payload=").append(payload); } } diff --git a/spring-web/src/test/java/org/springframework/web/filter/RequestLoggingFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/RequestLoggingFilterTests.java index fafd53bacd9e..ba4942358b4e 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/RequestLoggingFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/RequestLoggingFilterTests.java @@ -16,11 +16,9 @@ package org.springframework.web.filter; -import java.io.IOException; import java.nio.charset.StandardCharsets; import javax.servlet.FilterChain; -import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; @@ -30,6 +28,7 @@ import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; +import org.springframework.mock.web.test.MockHttpSession; import org.springframework.util.FileCopyUtils; import org.springframework.web.util.ContentCachingRequestWrapper; import org.springframework.web.util.WebUtils; @@ -46,10 +45,78 @@ public class RequestLoggingFilterTests { private final MyRequestLoggingFilter filter = new MyRequestLoggingFilter(); + @Test + public void defaultPrefix() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + FilterChain filterChain = new NoOpFilterChain(); + filter.doFilter(request, response, filterChain); + + assertThat(filter.beforeRequestMessage).startsWith(AbstractRequestLoggingFilter.DEFAULT_BEFORE_MESSAGE_PREFIX); + assertThat(filter.afterRequestMessage).startsWith(AbstractRequestLoggingFilter.DEFAULT_AFTER_MESSAGE_PREFIX); + } + + @Test + public void customPrefix() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + filter.setBeforeMessagePrefix("Before prefix: "); + filter.setAfterMessagePrefix("After prefix: "); + + FilterChain filterChain = new NoOpFilterChain(); + filter.doFilter(request, response, filterChain); + + assertThat(filter.beforeRequestMessage).startsWith("Before prefix: "); + assertThat(filter.afterRequestMessage).startsWith("After prefix: "); + } + + @Test + public void defaultSuffix() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + FilterChain filterChain = new NoOpFilterChain(); + filter.doFilter(request, response, filterChain); + + assertThat(filter.beforeRequestMessage).endsWith(AbstractRequestLoggingFilter.DEFAULT_BEFORE_MESSAGE_SUFFIX); + assertThat(filter.afterRequestMessage).endsWith(AbstractRequestLoggingFilter.DEFAULT_AFTER_MESSAGE_SUFFIX); + } + + @Test + public void customSuffix() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + filter.setBeforeMessageSuffix("}"); + filter.setAfterMessageSuffix(")"); + + FilterChain filterChain = new NoOpFilterChain(); + filter.doFilter(request, response, filterChain); + + assertThat(filter.beforeRequestMessage).endsWith("}"); + assertThat(filter.afterRequestMessage).endsWith(")"); + } + + @Test + public void method() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("PATCH", "/hotels"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + filter.setBeforeMessagePrefix(""); + filter.setAfterMessagePrefix(""); + + FilterChain filterChain = new NoOpFilterChain(); + filter.doFilter(request, response, filterChain); + + assertThat(filter.beforeRequestMessage).startsWith("PATCH"); + assertThat(filter.afterRequestMessage).startsWith("PATCH"); + } @Test public void uri() throws Exception { - final MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); request.setQueryString("booking=42"); @@ -57,13 +124,11 @@ public void uri() throws Exception { FilterChain filterChain = new NoOpFilterChain(); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.contains("uri=/hotel")).isTrue(); - assertThat(filter.beforeRequestMessage.contains("booking=42")).isFalse(); + assertThat(filter.beforeRequestMessage).contains("/hotel"); + assertThat(filter.beforeRequestMessage).doesNotContain("booking=42"); - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("uri=/hotel")).isTrue(); - assertThat(filter.afterRequestMessage.contains("booking=42")).isFalse(); + assertThat(filter.afterRequestMessage).contains("/hotel"); + assertThat(filter.afterRequestMessage).doesNotContain("booking=42"); } @Test @@ -78,11 +143,8 @@ public void queryStringIncluded() throws Exception { FilterChain filterChain = new NoOpFilterChain(); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.contains("[uri=/hotels?booking=42]")).isTrue(); - - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("[uri=/hotels?booking=42]")).isTrue(); + assertThat(filter.beforeRequestMessage).contains("/hotels?booking=42"); + assertThat(filter.afterRequestMessage).contains("/hotels?booking=42"); } @Test @@ -95,16 +157,59 @@ public void noQueryStringAvailable() throws Exception { FilterChain filterChain = new NoOpFilterChain(); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.contains("[uri=/hotels]")).isTrue(); + assertThat(filter.beforeRequestMessage).contains("/hotels]"); + assertThat(filter.afterRequestMessage).contains("/hotels]"); + } + + @Test + public void client() throws Exception { + filter.setIncludeClientInfo(true); - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("[uri=/hotels]")).isTrue(); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + request.setRemoteAddr("4.2.2.2"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + FilterChain filterChain = new NoOpFilterChain(); + filter.doFilter(request, response, filterChain); + + assertThat(filter.beforeRequestMessage).contains("client=4.2.2.2"); + assertThat(filter.afterRequestMessage).contains("client=4.2.2.2"); + } + + @Test + public void session() throws Exception { + filter.setIncludeClientInfo(true); + + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpSession session = new MockHttpSession(null, "42"); + request.setSession(session); + MockHttpServletResponse response = new MockHttpServletResponse(); + + FilterChain filterChain = new NoOpFilterChain(); + filter.doFilter(request, response, filterChain); + + assertThat(filter.beforeRequestMessage).contains("session=42"); + assertThat(filter.afterRequestMessage).contains("session=42"); + } + + @Test + public void user() throws Exception { + filter.setIncludeClientInfo(true); + + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + request.setRemoteUser("Arthur"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + FilterChain filterChain = new NoOpFilterChain(); + filter.doFilter(request, response, filterChain); + + assertThat(filter.beforeRequestMessage).contains("user=Arthur"); + assertThat(filter.afterRequestMessage).contains("user=Arthur"); } @Test public void headers() throws Exception { - final MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); request.setContentType("application/json"); request.addHeader("token", "123"); MockHttpServletResponse response = new MockHttpServletResponse(); @@ -114,21 +219,18 @@ public void headers() throws Exception { filter.setHeaderPredicate(name -> !name.equalsIgnoreCase("token")); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage).isEqualTo("Before request [uri=/hotels;headers=[Content-Type:\"application/json\", token:\"masked\"]]"); - - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage).isEqualTo("After request [uri=/hotels;headers=[Content-Type:\"application/json\", token:\"masked\"]]"); + assertThat(filter.beforeRequestMessage).isEqualTo("Before request [POST /hotels, headers=[Content-Type:\"application/json\", token:\"masked\"]]"); + assertThat(filter.afterRequestMessage).isEqualTo("After request [POST /hotels, headers=[Content-Type:\"application/json\", token:\"masked\"]]"); } @Test public void payloadInputStream() throws Exception { filter.setIncludePayload(true); - final MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); - final byte[] requestBody = "Hello World".getBytes(StandardCharsets.UTF_8); + byte[] requestBody = "Hello World".getBytes(StandardCharsets.UTF_8); request.setContent(requestBody); FilterChain filterChain = (filterRequest, filterResponse) -> { @@ -139,18 +241,17 @@ public void payloadInputStream() throws Exception { filter.doFilter(request, response, filterChain); - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("Hello World")).isTrue(); + assertThat(filter.afterRequestMessage).contains("Hello World"); } @Test public void payloadReader() throws Exception { filter.setIncludePayload(true); - final MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); - final String requestBody = "Hello World"; + String requestBody = "Hello World"; request.setContent(requestBody.getBytes(StandardCharsets.UTF_8)); FilterChain filterChain = (filterRequest, filterResponse) -> { @@ -161,8 +262,7 @@ public void payloadReader() throws Exception { filter.doFilter(request, response, filterChain); - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains(requestBody)).isTrue(); + assertThat(filter.afterRequestMessage).contains(requestBody); } @Test @@ -170,10 +270,10 @@ public void payloadMaxLength() throws Exception { filter.setIncludePayload(true); filter.setMaxPayloadLength(3); - final MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); - final byte[] requestBody = "Hello World".getBytes(StandardCharsets.UTF_8); + byte[] requestBody = "Hello World".getBytes(StandardCharsets.UTF_8); request.setContent(requestBody); FilterChain filterChain = (filterRequest, filterResponse) -> { @@ -187,9 +287,53 @@ public void payloadMaxLength() throws Exception { filter.doFilter(request, response, filterChain); - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("Hel")).isTrue(); - assertThat(filter.afterRequestMessage.contains("Hello World")).isFalse(); + assertThat(filter.afterRequestMessage).contains("Hel"); + assertThat(filter.afterRequestMessage).doesNotContain("Hello World"); + } + + @Test + public void allOptions() throws Exception { + filter.setIncludeQueryString(true); + filter.setIncludeClientInfo(true); + filter.setIncludeHeaders(true); + filter.setIncludePayload(true); + + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + request.setQueryString("booking=42"); + request.setRemoteAddr("4.2.2.2"); + request.setSession(new MockHttpSession(null, "42")); + request.setRemoteUser("Arthur"); + request.setContentType("application/json"); + String requestBody = "{\"msg\": \"Hello World\"}"; + request.setContent(requestBody.getBytes(StandardCharsets.UTF_8)); + MockHttpServletResponse response = new MockHttpServletResponse(); + + FilterChain filterChain = (filterRequest, filterResponse) -> { + ((HttpServletResponse) filterResponse).setStatus(HttpServletResponse.SC_OK); + String buf = FileCopyUtils.copyToString(filterRequest.getReader()); + assertThat(buf).isEqualTo(requestBody); + }; + + filter.doFilter(request, response, filterChain); + + assertThat(filter.beforeRequestMessage) + .isEqualTo("Before request [" + + "POST /hotels?booking=42" + + ", client=4.2.2.2" + + ", session=42" + + ", user=Arthur" + + ", headers=[Content-Type:\"application/json;charset=ISO-8859-1\", Content-Length:\"22\"]" + + "]"); + + assertThat(filter.afterRequestMessage) + .isEqualTo("After request [" + + "POST /hotels?booking=42" + + ", client=4.2.2.2" + + ", session=42" + + ", user=Arthur" + + ", headers=[Content-Type:\"application/json;charset=ISO-8859-1\", Content-Length:\"22\"]" + + ", payload={\"msg\": \"Hello World\"}" + + "]"); } @@ -214,7 +358,7 @@ protected void afterRequest(HttpServletRequest request, String message) { private static class NoOpFilterChain implements FilterChain { @Override - public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { + public void doFilter(ServletRequest request, ServletResponse response) { } }