Skip to content

Commit 607dba9

Browse files
committed
Only access parameters in WebRequestTraceFilter when they are included
Previously, WebRequestTraceFilter would call request.getParameterMap() before deciding whether or not the parameters should be included in the trace. For a POST request, this had the unwanted side-effect of always reading the request body. This commit updates WebRequestTraceFilter so that it checks that parameters are to be included in the trace before calling request.getParameterMap() Closes gh-5089
1 parent 815da8b commit 607dba9

File tree

2 files changed

+11
-3
lines changed

2 files changed

+11
-3
lines changed

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/WebRequestTraceFilter.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
*
4646
* @author Dave Syer
4747
* @author Wallace Wadge
48+
* @author Andy Wilkinson
4849
*/
4950
public class WebRequestTraceFilter extends OncePerRequestFilter implements Ordered {
5051

@@ -135,7 +136,9 @@ protected Map<String, Object> getTrace(HttpServletRequest request) {
135136
add(trace, Include.CONTEXT_PATH, "contextPath", request.getContextPath());
136137
add(trace, Include.USER_PRINCIPAL, "userPrincipal",
137138
(userPrincipal == null ? null : userPrincipal.getName()));
138-
add(trace, Include.PARAMETERS, "parameters", request.getParameterMap());
139+
if (isIncluded(Include.PARAMETERS)) {
140+
trace.put("parameters", request.getParameterMap());
141+
}
139142
add(trace, Include.QUERY_STRING, "query", request.getQueryString());
140143
add(trace, Include.AUTH_TYPE, "authType", request.getAuthType());
141144
add(trace, Include.REMOTE_ADDRESS, "remoteAddress", request.getRemoteAddr());

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/trace/WebRequestTraceFilterTests.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2015 the original author or authors.
2+
* Copyright 2012-2016 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -39,13 +39,17 @@
3939

4040
import static org.junit.Assert.assertEquals;
4141
import static org.junit.Assert.assertTrue;
42+
import static org.mockito.Mockito.spy;
43+
import static org.mockito.Mockito.times;
44+
import static org.mockito.Mockito.verify;
4245

4346
/**
4447
* Tests for {@link WebRequestTraceFilter}.
4548
*
4649
* @author Dave Syer
4750
* @author Wallace Wadge
4851
* @author Phillip Webb
52+
* @author Andy Wilkinson
4953
*/
5054
public class WebRequestTraceFilterTests {
5155

@@ -59,13 +63,14 @@ public class WebRequestTraceFilterTests {
5963
@Test
6064
@SuppressWarnings("unchecked")
6165
public void filterAddsTraceWithDefaultIncludes() {
62-
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo");
66+
MockHttpServletRequest request = spy(new MockHttpServletRequest("GET", "/foo"));
6367
request.addHeader("Accept", "application/json");
6468
Map<String, Object> trace = this.filter.getTrace(request);
6569
assertEquals("GET", trace.get("method"));
6670
assertEquals("/foo", trace.get("path"));
6771
Map<String, Object> map = (Map<String, Object>) trace.get("headers");
6872
assertEquals("{Accept=application/json}", map.get("request").toString());
73+
verify(request, times(0)).getParameterMap();
6974
}
7075

7176
@Test

0 commit comments

Comments
 (0)