From 3fdca2fc81bc16b615303769c73596307e50fc05 Mon Sep 17 00:00:00 2001 From: Napster Date: Sat, 14 Sep 2019 02:16:43 +0200 Subject: [PATCH] Log HTTP method in logging filters and revise log message format --- .../filter/AbstractRequestLoggingFilter.java | 13 +- .../web/filter/RequestLoggingFilterTests.java | 167 ++++++++++++++++-- 2 files changed, 162 insertions(+), 18 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..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,20 +16,20 @@ 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; import javax.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; 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; @@ -44,8 +44,96 @@ */ public class RequestLoggingFilterTests { - private final MyRequestLoggingFilter filter = new MyRequestLoggingFilter(); + private MyRequestLoggingFilter filter = new MyRequestLoggingFilter(); + @BeforeEach + 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"); + 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 +146,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 +167,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 +184,65 @@ 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("/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("[uri=/hotels]")).isTrue(); + assertThat(filter.afterRequestMessage.contains("user=Arthur")).isTrue(); } @Test @@ -115,10 +258,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 @@ -214,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) { } }