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
  • Loading branch information
@schnapster
schnapster committed Sep 14, 2019
1 parent 957f0fa commit 3fdca2f
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 18 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
167 changes: 155 additions & 12 deletions spring-web/src/test/java/org/springframework/web/filter/RequestLoggingFilterTests.java
View file
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -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();
}

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

Expand Down

0 comments on commit 3fdca2f

Please sign in to comment.