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 2 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
167 changes: 155 additions & 12 deletions spring-web/src/test/java/org/springframework/web/filter/RequestLoggingFilterTests.java
View file
Open in desktop
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();
schnapster marked this conversation as resolved.
Show resolved Hide resolved
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");
schnapster marked this conversation as resolved.
Show resolved Hide resolved
MockHttpServletResponse response = new MockHttpServletResponse();

FilterChain filterChain = new NoOpFilterChain();
filter.doFilter(request, response, filterChain);

assertThat(filter.beforeRequestMessage).isNotNull();
schnapster marked this conversation as resolved.
Show resolved Hide resolved
assertThat(filter.beforeRequestMessage.startsWith(AbstractRequestLoggingFilter.DEFAULT_BEFORE_MESSAGE_PREFIX)).isTrue();
schnapster marked this conversation as resolved.
Show resolved Hide resolved

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();
schnapster marked this conversation as resolved.
Show resolved Hide resolved
assertThat(filter.beforeRequestMessage.contains("uri=/hotel")).isTrue();
assertThat(filter.beforeRequestMessage.contains("/hotel")).isTrue();
schnapster marked this conversation as resolved.
Show resolved Hide resolved
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