From fde5e1b6a53411005c50fc54f7b4bf6224b584e6 Mon Sep 17 00:00:00 2001 From: Venil Noronha Date: Tue, 24 May 2016 09:59:56 +0530 Subject: [PATCH 1/2] Add ability to filter cookies in trace data See gh-6018 --- .../boot/actuate/trace/TraceProperties.java | 9 +++++- .../actuate/trace/WebRequestTraceFilter.java | 13 ++++++-- .../trace/WebRequestTraceFilterTests.java | 30 +++++++++++++++++++ 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/TraceProperties.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/TraceProperties.java index 13184a50a6..fd38d8d350 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/TraceProperties.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/TraceProperties.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. @@ -28,6 +28,7 @@ import org.springframework.boot.context.properties.ConfigurationProperties; * * @author Wallace Wadge * @author Phillip Webb + * @author Venil Noronha * @since 1.3.0 */ @ConfigurationProperties(prefix = "management.trace") @@ -39,6 +40,7 @@ public class TraceProperties { Set defaultIncludes = new LinkedHashSet(); defaultIncludes.add(Include.REQUEST_HEADERS); defaultIncludes.add(Include.RESPONSE_HEADERS); + defaultIncludes.add(Include.COOKIES); defaultIncludes.add(Include.ERRORS); DEFAULT_INCLUDES = Collections.unmodifiableSet(defaultIncludes); } @@ -71,6 +73,11 @@ public class TraceProperties { */ RESPONSE_HEADERS, + /** + * Include Cookie in request and Set-Cookie in response headers. + */ + COOKIES, + /** * Include errors (if any). */ 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 d1c86f0d36..c716de6412 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 @@ -48,6 +48,7 @@ import org.springframework.web.filter.OncePerRequestFilter; * @author Dave Syer * @author Wallace Wadge * @author Andy Wilkinson + * @author Venil Noronha */ public class WebRequestTraceFilter extends OncePerRequestFilter implements Ordered { @@ -122,7 +123,11 @@ public class WebRequestTraceFilter extends OncePerRequestFilter implements Order trace.put("path", request.getRequestURI()); trace.put("headers", headers); if (isIncluded(Include.REQUEST_HEADERS)) { - headers.put("request", getRequestHeaders(request)); + Map requestHeaders = getRequestHeaders(request); + if (!isIncluded(Include.COOKIES)) { + requestHeaders.remove("Cookie"); + } + headers.put("request", requestHeaders); } add(trace, Include.PATH_INFO, "pathInfo", request.getPathInfo()); add(trace, Include.PATH_TRANSLATED, "pathTranslated", @@ -169,7 +174,11 @@ public class WebRequestTraceFilter extends OncePerRequestFilter implements Order protected void enhanceTrace(Map trace, HttpServletResponse response) { if (isIncluded(Include.RESPONSE_HEADERS)) { Map headers = (Map) trace.get("headers"); - headers.put("response", getResponseHeaders(response)); + Map responseHeaders = getResponseHeaders(response); + if (!isIncluded(Include.COOKIES)) { + responseHeaders.remove("Set-Cookie"); + } + headers.put("response", responseHeaders); } } 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 db0a32b7ab..4f4db9ea1a 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 @@ -50,6 +50,7 @@ import static org.mockito.Mockito.verify; * @author Wallace Wadge * @author Phillip Webb * @author Andy Wilkinson + * @author Venil Noronha */ public class WebRequestTraceFilterTests { @@ -153,6 +154,35 @@ public class WebRequestTraceFilterTests { assertThat(headers.get("response") == null).isTrue(); } + @Test + @SuppressWarnings({ "unchecked" }) + public void filterDoesNotAddRequestCookiesWithCookiesExclude() + throws ServletException, IOException { + this.properties.setInclude(Collections.singleton(Include.REQUEST_HEADERS)); + MockHttpServletRequest request = spy(new MockHttpServletRequest("GET", "/foo")); + request.addHeader("Accept", "application/json"); + request.addHeader("Cookie", "testCookie=testValue;"); + Map map = (Map) this.filter.getTrace(request) + .get("headers"); + assertThat(map.get("request").toString()).isEqualTo("{Accept=application/json}"); + } + + @Test + @SuppressWarnings({ "unchecked" }) + public void filterDoesNotAddResponseCookiesWithCookiesExclude() + throws ServletException, IOException { + this.properties.setInclude(Collections.singleton(Include.RESPONSE_HEADERS)); + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); + MockHttpServletResponse response = new MockHttpServletResponse(); + response.addHeader("Content-Type", "application/json"); + response.addHeader("Set-Cookie", "testCookie=testValue;"); + Map trace = this.filter.getTrace(request); + this.filter.enhanceTrace(trace, response); + Map map = (Map) trace.get("headers"); + assertThat(map.get("response").toString()) + .isEqualTo("{Content-Type=application/json, status=200}"); + } + @Test public void filterHasResponseStatus() { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); From 84b2ff5c38e11ebfc908ce2b99f1c0ab23388351 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 27 Jun 2016 16:39:20 +0200 Subject: [PATCH 2/2] Polish "Add ability to filter cookies in trace data" Closes gh-6018 --- .../boot/actuate/trace/TraceProperties.java | 5 +++-- .../actuate/trace/WebRequestTraceFilter.java | 18 ++++++++---------- .../trace/WebRequestTraceFilterTests.java | 11 ++++++----- .../appendix-application-properties.adoc | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/TraceProperties.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/TraceProperties.java index fd38d8d350..3233ead562 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/TraceProperties.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/trace/TraceProperties.java @@ -46,7 +46,8 @@ public class TraceProperties { } /** - * Items to be included in the trace. Defaults to request/response headers and errors. + * Items to be included in the trace. Defaults to request/response headers (including cookies) + * and errors. */ private Set include = new HashSet(DEFAULT_INCLUDES); @@ -74,7 +75,7 @@ public class TraceProperties { RESPONSE_HEADERS, /** - * Include Cookie in request and Set-Cookie in response headers. + * Include "Cookie" in request and "Set-Cookie" in response headers. */ COOKIES, 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 c716de6412..f3de956623 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 @@ -123,11 +123,7 @@ public class WebRequestTraceFilter extends OncePerRequestFilter implements Order trace.put("path", request.getRequestURI()); trace.put("headers", headers); if (isIncluded(Include.REQUEST_HEADERS)) { - Map requestHeaders = getRequestHeaders(request); - if (!isIncluded(Include.COOKIES)) { - requestHeaders.remove("Cookie"); - } - headers.put("request", requestHeaders); + headers.put("request", getRequestHeaders(request)); } add(trace, Include.PATH_INFO, "pathInfo", request.getPathInfo()); add(trace, Include.PATH_TRANSLATED, "pathTranslated", @@ -167,6 +163,9 @@ public class WebRequestTraceFilter extends OncePerRequestFilter implements Order } headers.put(name, value); } + if (!isIncluded(Include.COOKIES)) { + headers.remove("Cookie"); + } return headers; } @@ -174,11 +173,7 @@ public class WebRequestTraceFilter extends OncePerRequestFilter implements Order protected void enhanceTrace(Map trace, HttpServletResponse response) { if (isIncluded(Include.RESPONSE_HEADERS)) { Map headers = (Map) trace.get("headers"); - Map responseHeaders = getResponseHeaders(response); - if (!isIncluded(Include.COOKIES)) { - responseHeaders.remove("Set-Cookie"); - } - headers.put("response", responseHeaders); + headers.put("response", getResponseHeaders(response)); } } @@ -188,6 +183,9 @@ public class WebRequestTraceFilter extends OncePerRequestFilter implements Order String value = response.getHeader(header); headers.put(header, value); } + if (!isIncluded(Include.COOKIES)) { + headers.remove("Set-Cookie"); + } headers.put("status", "" + response.getStatus()); return headers; } 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 4f4db9ea1a..6722146555 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 @@ -28,7 +28,6 @@ import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; -import javax.servlet.http.Cookie; import org.junit.Test; @@ -51,6 +50,7 @@ import static org.mockito.Mockito.verify; * @author Phillip Webb * @author Andy Wilkinson * @author Venil Noronha + * @author Stephane Nicoll */ public class WebRequestTraceFilterTests { @@ -80,6 +80,7 @@ public class WebRequestTraceFilterTests { this.properties.setInclude(EnumSet.allOf(Include.class)); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); request.addHeader("Accept", "application/json"); + request.addHeader("Cookie", "testCookie=testValue;"); request.setContextPath("some.context.path"); request.setContent("Hello, World!".getBytes()); request.setRemoteAddr("some.remote.addr"); @@ -89,8 +90,6 @@ public class WebRequestTraceFilterTests { String url = tmp.toURI().toURL().toString(); request.setPathInfo(url); tmp.deleteOnExit(); - Cookie cookie = new Cookie("testCookie", "testValue"); - request.setCookies(cookie); request.setAuthType("authType"); Principal principal = new Principal() { @@ -103,6 +102,7 @@ public class WebRequestTraceFilterTests { request.setUserPrincipal(principal); MockHttpServletResponse response = new MockHttpServletResponse(); response.addHeader("Content-Type", "application/json"); + response.addHeader("Set-Cookie", "a=b"); this.filter.doFilterInternal(request, response, new FilterChain() { @Override @@ -121,7 +121,7 @@ public class WebRequestTraceFilterTests { Map map = (Map) trace.get("headers"); assertThat(map.get("response").toString()) - .isEqualTo("{Content-Type=application/json, status=200}"); + .isEqualTo("{Content-Type=application/json, Set-Cookie=a=b, status=200}"); assertThat(trace.get("method")).isEqualTo("GET"); assertThat(trace.get("path")).isEqualTo("/foo"); assertThat(((String[]) ((Map) trace.get("parameters")).get("param"))[0]) @@ -132,7 +132,8 @@ public class WebRequestTraceFilterTests { assertThat(trace.get("contextPath")).isEqualTo("some.context.path"); assertThat(trace.get("pathInfo")).isEqualTo(url); assertThat(trace.get("authType")).isEqualTo("authType"); - assertThat(map.get("request").toString()).isEqualTo("{Accept=application/json}"); + assertThat(map.get("request").toString()) + .isEqualTo("{Accept=application/json, Cookie=testCookie=testValue;}"); } @Test diff --git a/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc b/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc index 5bfa9e4af8..e0d915265c 100644 --- a/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc +++ b/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc @@ -1072,7 +1072,7 @@ content into your application; rather pick only the properties that you need. management.shell.telnet.port=5000 # Telnet port. # TRACING ({sc-spring-boot-actuator}/trace/TraceProperties.{sc-ext}[TraceProperties]) - management.trace.include=request-headers,response-headers,errors # Items to be included in the trace. + management.trace.include=request-headers,response-headers,cookies,errors # Items to be included in the trace. # METRICS EXPORT ({sc-spring-boot-actuator}/metrics/export/MetricExportProperties.{sc-ext}[MetricExportProperties]) spring.metrics.export.aggregate.key-pattern= # Pattern that tells the aggregator what to do with the keys from the source repository.