Log HTTP method in logging filters and revise log message format

Closes gh-23567

spring-web/src/main/java/org/springframework/web/filter/AbstractRequestLoggingFilter.java

@@ -322,7 +322,8 @@ public abstract class AbstractRequestLoggingFilter extends OncePerRequestFilter
 	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 @@ public abstract class AbstractRequestLoggingFilter extends OncePerRequestFilter
 		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 @@ public abstract class AbstractRequestLoggingFilter extends OncePerRequestFilter
 					}
 				}
 			}
-			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);
 			}
 		}
 

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.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;
@@ -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 class RequestLoggingFilterTests {
 		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 class RequestLoggingFilterTests {
 		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 class RequestLoggingFilterTests {
 		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 class RequestLoggingFilterTests {
 		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 class RequestLoggingFilterTests {
 
 		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 class RequestLoggingFilterTests {
 
 		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 class RequestLoggingFilterTests {
 		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 class RequestLoggingFilterTests {
 
 		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 @@ public class RequestLoggingFilterTests {
 	private static class NoOpFilterChain implements FilterChain {
 
 		@Override
-		public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException {
+		public void doFilter(ServletRequest request, ServletResponse response) {
 		}
 	}