Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Log HTTP method in logging filters and revise log message format #23567

Closed
wants to merge 4 commits into from
Closed

Log HTTP method in logging filters and revise log message format #23567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e9ddad0
Log HTTP method in logging filters and revise log message format
schnapster Sep 14, 2019
dd041ce
Add tests for logging prefixes, suffixes, client, session & user
schnapster Sep 14, 2019
ae469f4
Clean up finals, take advantage of more expressive assertions
schnapster Sep 14, 2019
4b1bcb4
Add test for a fully logged request
schnapster Sep 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 7 additions & 6 deletions spring-web/src/main/java/org/springframework/web/filter/AbstractRequestLoggingFilter.java
View file
Open in desktop
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
Open in desktop
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();
schnapster marked this conversation as resolved.
Show resolved Hide resolved

@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