From 607dba97f8e3d69517924431678179b003d3d4a5 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 12 Feb 2016 10:26:23 +0000 Subject: [PATCH] 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 --- .../boot/actuate/trace/WebRequestTraceFilter.java | 5 ++++- .../boot/actuate/trace/WebRequestTraceFilterTests.java | 9 +++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/WebRequestTraceFilter.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/WebRequestTraceFilter.java index 0e47397833f..9ddf8ab7093 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/WebRequestTraceFilter.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/WebRequestTraceFilter.java @@ -45,6 +45,7 @@ import org.springframework.web.filter.OncePerRequestFilter; * * @author Dave Syer * @author Wallace Wadge + * @author Andy Wilkinson */ public class WebRequestTraceFilter extends OncePerRequestFilter implements Ordered { @@ -135,7 +136,9 @@ public class WebRequestTraceFilter extends OncePerRequestFilter implements Order add(trace, Include.CONTEXT_PATH, "contextPath", request.getContextPath()); add(trace, Include.USER_PRINCIPAL, "userPrincipal", (userPrincipal == null ? null : userPrincipal.getName())); - add(trace, Include.PARAMETERS, "parameters", request.getParameterMap()); + if (isIncluded(Include.PARAMETERS)) { + trace.put("parameters", request.getParameterMap()); + } add(trace, Include.QUERY_STRING, "query", request.getQueryString()); add(trace, Include.AUTH_TYPE, "authType", request.getAuthType()); add(trace, Include.REMOTE_ADDRESS, "remoteAddress", request.getRemoteAddr()); diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/trace/WebRequestTraceFilterTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/trace/WebRequestTraceFilterTests.java index c122f50c255..a69ee7c349a 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/trace/WebRequestTraceFilterTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/trace/WebRequestTraceFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * Copyright 2012-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -39,6 +39,9 @@ import org.springframework.mock.web.MockHttpServletResponse; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; /** * Tests for {@link WebRequestTraceFilter}. @@ -46,6 +49,7 @@ import static org.junit.Assert.assertTrue; * @author Dave Syer * @author Wallace Wadge * @author Phillip Webb + * @author Andy Wilkinson */ public class WebRequestTraceFilterTests { @@ -59,13 +63,14 @@ public class WebRequestTraceFilterTests { @Test @SuppressWarnings("unchecked") public void filterAddsTraceWithDefaultIncludes() { - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); + MockHttpServletRequest request = spy(new MockHttpServletRequest("GET", "/foo")); request.addHeader("Accept", "application/json"); Map trace = this.filter.getTrace(request); assertEquals("GET", trace.get("method")); assertEquals("/foo", trace.get("path")); Map map = (Map) trace.get("headers"); assertEquals("{Accept=application/json}", map.get("request").toString()); + verify(request, times(0)).getParameterMap(); } @Test