Skip to content

Commit

Permalink
Log HTTP method in logging filters and revise log message format
Browse files Browse the repository at this point in the history 
Closes gh-23567
  • Loading branch information
@schnapster @sbrannen
schnapster authored and sbrannen committed Sep 16, 2019
1 parent a65fd34 commit 33cee06
Show file tree
Hide file tree
Showing 2 changed files with 189 additions and 44 deletions.
13 changes: 7 additions & 6 deletions spring-web/src/main/java/org/springframework/web/filter/AbstractRequestLoggingFilter.java
View file
Expand Up @@ -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();
Expand All @@ -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);
}
}

Expand All @@ -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);
}
}

Expand Down
220 changes: 182 additions & 38 deletions spring-web/src/test/java/org/springframework/web/filter/RequestLoggingFilterTests.java
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -46,24 +45,90 @@ 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");

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
Expand All @@ -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
Expand All @@ -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();
Expand All @@ -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) -> {
Expand All @@ -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) -> {
Expand All @@ -161,19 +262,18 @@ 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
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) -> {
Expand All @@ -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\"}"
+ "]");
}


Expand All @@ -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) {
}
}

Expand Down

0 comments on commit 33cee06

Please sign in to comment.