From e9ddad0374c7562f55f2e7e035a908c99431573f Mon Sep 17 00:00:00 2001 From: Napster Date: Sat, 14 Sep 2019 02:30:43 +0200 Subject: [PATCH 1/4] Log HTTP method in logging filters and revise log message format --- .../filter/AbstractRequestLoggingFilter.java | 13 +++--- .../web/filter/RequestLoggingFilterTests.java | 41 +++++++++++++++---- 2 files changed, 39 insertions(+), 15 deletions(-) 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..bcce752c7ae8 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 @@ -26,6 +26,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.mock.web.test.MockHttpServletRequest; @@ -44,8 +45,30 @@ */ public class RequestLoggingFilterTests { - private final MyRequestLoggingFilter filter = new MyRequestLoggingFilter(); + private MyRequestLoggingFilter filter = new MyRequestLoggingFilter(); + @BeforeEach + public void reset() { + filter = new MyRequestLoggingFilter(); + } + + @Test + public void method() throws Exception { + final 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).isNotNull(); + assertThat(filter.beforeRequestMessage.startsWith("PATCH")).isTrue(); + + assertThat(filter.afterRequestMessage).isNotNull(); + assertThat(filter.afterRequestMessage.startsWith("PATCH")).isTrue(); + } @Test public void uri() throws Exception { @@ -58,11 +81,11 @@ public void uri() throws Exception { filter.doFilter(request, response, filterChain); assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.contains("uri=/hotel")).isTrue(); + assertThat(filter.beforeRequestMessage.contains("/hotel")).isTrue(); assertThat(filter.beforeRequestMessage.contains("booking=42")).isFalse(); assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("uri=/hotel")).isTrue(); + assertThat(filter.afterRequestMessage.contains("/hotel")).isTrue(); assertThat(filter.afterRequestMessage.contains("booking=42")).isFalse(); } @@ -79,10 +102,10 @@ public void queryStringIncluded() throws Exception { filter.doFilter(request, response, filterChain); assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.contains("[uri=/hotels?booking=42]")).isTrue(); + assertThat(filter.beforeRequestMessage.contains("/hotels?booking=42")).isTrue(); assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("[uri=/hotels?booking=42]")).isTrue(); + assertThat(filter.afterRequestMessage.contains("/hotels?booking=42")).isTrue(); } @Test @@ -96,10 +119,10 @@ public void noQueryStringAvailable() throws Exception { filter.doFilter(request, response, filterChain); assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.contains("[uri=/hotels]")).isTrue(); + assertThat(filter.beforeRequestMessage.contains("/hotels]")).isTrue(); assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("[uri=/hotels]")).isTrue(); + assertThat(filter.afterRequestMessage.contains("/hotels]")).isTrue(); } @Test @@ -115,10 +138,10 @@ public void headers() throws Exception { 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.beforeRequestMessage).isEqualTo("Before request [POST /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.afterRequestMessage).isEqualTo("After request [POST /hotels, headers=[Content-Type:\"application/json\", token:\"masked\"]]"); } @Test From dd041ce537db0ab36e55b127a386049f777ddd93 Mon Sep 17 00:00:00 2001 From: Napster Date: Sat, 14 Sep 2019 02:34:43 +0200 Subject: [PATCH 2/4] Add tests for logging prefixes, suffixes, client, session & user --- .../web/filter/RequestLoggingFilterTests.java | 126 +++++++++++++++++- 1 file changed, 123 insertions(+), 3 deletions(-) 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 bcce752c7ae8..c72d911679f8 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; @@ -31,6 +29,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; @@ -52,6 +51,72 @@ public void reset() { filter = new MyRequestLoggingFilter(); } + @Test + public void defaultPrefix() throws Exception { + final MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + FilterChain filterChain = new NoOpFilterChain(); + filter.doFilter(request, response, filterChain); + + assertThat(filter.beforeRequestMessage).isNotNull(); + assertThat(filter.beforeRequestMessage.startsWith(AbstractRequestLoggingFilter.DEFAULT_BEFORE_MESSAGE_PREFIX)).isTrue(); + + assertThat(filter.afterRequestMessage).isNotNull(); + assertThat(filter.afterRequestMessage.startsWith(AbstractRequestLoggingFilter.DEFAULT_AFTER_MESSAGE_PREFIX)).isTrue(); + } + + @Test + public void customPrefix() throws Exception { + final 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).isNotNull(); + assertThat(filter.beforeRequestMessage.startsWith("Before prefix: ")).isTrue(); + + assertThat(filter.afterRequestMessage).isNotNull(); + assertThat(filter.afterRequestMessage.startsWith("After prefix: ")).isTrue(); + } + + @Test + public void defaultSuffix() throws Exception { + final MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + FilterChain filterChain = new NoOpFilterChain(); + filter.doFilter(request, response, filterChain); + + assertThat(filter.beforeRequestMessage).isNotNull(); + assertThat(filter.beforeRequestMessage.endsWith(AbstractRequestLoggingFilter.DEFAULT_BEFORE_MESSAGE_SUFFIX)).isTrue(); + + assertThat(filter.afterRequestMessage).isNotNull(); + assertThat(filter.afterRequestMessage.endsWith(AbstractRequestLoggingFilter.DEFAULT_AFTER_MESSAGE_SUFFIX)).isTrue(); + } + + @Test + public void customSuffix() throws Exception { + final 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).isNotNull(); + assertThat(filter.beforeRequestMessage.endsWith("}")).isTrue(); + + assertThat(filter.afterRequestMessage).isNotNull(); + assertThat(filter.afterRequestMessage.endsWith(")")).isTrue(); + } + @Test public void method() throws Exception { final MockHttpServletRequest request = new MockHttpServletRequest("PATCH", "/hotels"); @@ -125,6 +190,61 @@ public void noQueryStringAvailable() throws Exception { assertThat(filter.afterRequestMessage.contains("/hotels]")).isTrue(); } + @Test + public void client() throws Exception { + filter.setIncludeClientInfo(true); + + 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).isNotNull(); + assertThat(filter.beforeRequestMessage.contains("client=4.2.2.2")).isTrue(); + + assertThat(filter.afterRequestMessage).isNotNull(); + assertThat(filter.afterRequestMessage.contains("client=4.2.2.2")).isTrue(); + } + + @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).isNotNull(); + assertThat(filter.beforeRequestMessage.contains("session=42")).isTrue(); + + assertThat(filter.afterRequestMessage).isNotNull(); + assertThat(filter.afterRequestMessage.contains("session=42")).isTrue(); + } + + @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).isNotNull(); + assertThat(filter.beforeRequestMessage.contains("user=Arthur")).isTrue(); + + assertThat(filter.afterRequestMessage).isNotNull(); + assertThat(filter.afterRequestMessage.contains("user=Arthur")).isTrue(); + } + @Test public void headers() throws Exception { final MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); @@ -237,7 +357,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) { } } From ae469f4b31e0209ad16396cf18da6993db68b48d Mon Sep 17 00:00:00 2001 From: Napster Date: Sat, 14 Sep 2019 12:42:12 +0200 Subject: [PATCH 3/4] Clean up finals, take advantage of more expressive assertions --- .../web/filter/RequestLoggingFilterTests.java | 128 ++++++------------ 1 file changed, 42 insertions(+), 86 deletions(-) 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 c72d911679f8..1d96680ae134 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 @@ -24,7 +24,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.mock.web.test.MockHttpServletRequest; @@ -44,31 +43,23 @@ */ public class RequestLoggingFilterTests { - private MyRequestLoggingFilter filter = new MyRequestLoggingFilter(); - - @BeforeEach - public void reset() { - filter = new MyRequestLoggingFilter(); - } + private final MyRequestLoggingFilter filter = new MyRequestLoggingFilter(); @Test public void defaultPrefix() throws Exception { - final MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); FilterChain filterChain = new NoOpFilterChain(); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.startsWith(AbstractRequestLoggingFilter.DEFAULT_BEFORE_MESSAGE_PREFIX)).isTrue(); - - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.startsWith(AbstractRequestLoggingFilter.DEFAULT_AFTER_MESSAGE_PREFIX)).isTrue(); + assertThat(filter.beforeRequestMessage).startsWith(AbstractRequestLoggingFilter.DEFAULT_BEFORE_MESSAGE_PREFIX); + assertThat(filter.afterRequestMessage).startsWith(AbstractRequestLoggingFilter.DEFAULT_AFTER_MESSAGE_PREFIX); } @Test public void customPrefix() throws Exception { - final MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); filter.setBeforeMessagePrefix("Before prefix: "); @@ -77,31 +68,25 @@ public void customPrefix() throws Exception { FilterChain filterChain = new NoOpFilterChain(); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.startsWith("Before prefix: ")).isTrue(); - - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.startsWith("After prefix: ")).isTrue(); + assertThat(filter.beforeRequestMessage).startsWith("Before prefix: "); + assertThat(filter.afterRequestMessage).startsWith("After prefix: "); } @Test public void defaultSuffix() throws Exception { - final MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); FilterChain filterChain = new NoOpFilterChain(); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.endsWith(AbstractRequestLoggingFilter.DEFAULT_BEFORE_MESSAGE_SUFFIX)).isTrue(); - - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.endsWith(AbstractRequestLoggingFilter.DEFAULT_AFTER_MESSAGE_SUFFIX)).isTrue(); + assertThat(filter.beforeRequestMessage).endsWith(AbstractRequestLoggingFilter.DEFAULT_BEFORE_MESSAGE_SUFFIX); + assertThat(filter.afterRequestMessage).endsWith(AbstractRequestLoggingFilter.DEFAULT_AFTER_MESSAGE_SUFFIX); } @Test public void customSuffix() throws Exception { - final MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); filter.setBeforeMessageSuffix("}"); @@ -110,16 +95,13 @@ public void customSuffix() throws Exception { FilterChain filterChain = new NoOpFilterChain(); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.endsWith("}")).isTrue(); - - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.endsWith(")")).isTrue(); + assertThat(filter.beforeRequestMessage).endsWith("}"); + assertThat(filter.afterRequestMessage).endsWith(")"); } @Test public void method() throws Exception { - final MockHttpServletRequest request = new MockHttpServletRequest("PATCH", "/hotels"); + MockHttpServletRequest request = new MockHttpServletRequest("PATCH", "/hotels"); MockHttpServletResponse response = new MockHttpServletResponse(); filter.setBeforeMessagePrefix(""); @@ -128,16 +110,13 @@ public void method() throws Exception { FilterChain filterChain = new NoOpFilterChain(); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.startsWith("PATCH")).isTrue(); - - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.startsWith("PATCH")).isTrue(); + 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"); @@ -145,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("/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("/hotel")).isTrue(); - assertThat(filter.afterRequestMessage.contains("booking=42")).isFalse(); + assertThat(filter.afterRequestMessage).contains("/hotel"); + assertThat(filter.afterRequestMessage).doesNotContain("booking=42"); } @Test @@ -166,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("/hotels?booking=42")).isTrue(); - - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("/hotels?booking=42")).isTrue(); + assertThat(filter.beforeRequestMessage).contains("/hotels?booking=42"); + assertThat(filter.afterRequestMessage).contains("/hotels?booking=42"); } @Test @@ -183,11 +157,8 @@ public void noQueryStringAvailable() throws Exception { FilterChain filterChain = new NoOpFilterChain(); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.contains("/hotels]")).isTrue(); - - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("/hotels]")).isTrue(); + assertThat(filter.beforeRequestMessage).contains("/hotels]"); + assertThat(filter.afterRequestMessage).contains("/hotels]"); } @Test @@ -201,11 +172,8 @@ public void client() throws Exception { FilterChain filterChain = new NoOpFilterChain(); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.contains("client=4.2.2.2")).isTrue(); - - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("client=4.2.2.2")).isTrue(); + assertThat(filter.beforeRequestMessage).contains("client=4.2.2.2"); + assertThat(filter.afterRequestMessage).contains("client=4.2.2.2"); } @Test @@ -220,11 +188,8 @@ public void session() throws Exception { FilterChain filterChain = new NoOpFilterChain(); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.contains("session=42")).isTrue(); - - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("session=42")).isTrue(); + assertThat(filter.beforeRequestMessage).contains("session=42"); + assertThat(filter.afterRequestMessage).contains("session=42"); } @Test @@ -238,16 +203,13 @@ public void user() throws Exception { FilterChain filterChain = new NoOpFilterChain(); filter.doFilter(request, response, filterChain); - assertThat(filter.beforeRequestMessage).isNotNull(); - assertThat(filter.beforeRequestMessage.contains("user=Arthur")).isTrue(); - - assertThat(filter.afterRequestMessage).isNotNull(); - assertThat(filter.afterRequestMessage.contains("user=Arthur")).isTrue(); + 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(); @@ -257,10 +219,7 @@ 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 [POST /hotels, headers=[Content-Type:\"application/json\", token:\"masked\"]]"); - - assertThat(filter.afterRequestMessage).isNotNull(); assertThat(filter.afterRequestMessage).isEqualTo("After request [POST /hotels, headers=[Content-Type:\"application/json\", token:\"masked\"]]"); } @@ -268,10 +227,10 @@ public void headers() throws Exception { 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) -> { @@ -282,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) -> { @@ -304,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 @@ -313,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) -> { @@ -330,9 +287,8 @@ 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"); } From 4b1bcb43fbcd184959a735bc0255647b8717c99d Mon Sep 17 00:00:00 2001 From: Napster Date: Sat, 14 Sep 2019 13:15:13 +0200 Subject: [PATCH 4/4] Add test for a fully logged request --- .../web/filter/RequestLoggingFilterTests.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) 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 1d96680ae134..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 @@ -291,6 +291,51 @@ public void payloadMaxLength() throws Exception { 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\"}" + + "]"); + } + private static class MyRequestLoggingFilter extends AbstractRequestLoggingFilter {