Track customization of sensitive headers so route can be cleared

When the users explicitly sets the sensitive headers of a rroute to
empty, it means they should be empty, not the global defaults.

Fixes gh-1012

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/Route.java

@@ -36,6 +36,7 @@ public class Route {
 		this.retryable = retryable;
 		this.sensitiveHeaders = new LinkedHashSet<>();
 		if (ignoredHeaders != null) {
+			this.customSensitiveHeaders = true;
 			for (String header : ignoredHeaders) {
 				this.sensitiveHeaders.add(header.toLowerCase());
 			}
@@ -56,4 +57,10 @@ public class Route {
 
 	private Set<String> sensitiveHeaders = new LinkedHashSet<>();
 
+	private boolean customSensitiveHeaders;
+
+	public boolean isCustomSensitiveHeaders() {
+		return this.customSensitiveHeaders;
+	}
+
 }

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java

@@ -90,9 +90,10 @@ public class SimpleRouteLocator implements RouteLocator {
 
 		log.debug("servletPath=" + this.dispatcherServletPath);
 		log.debug("zuulServletPath=" + this.zuulServletPath);
-		log.debug("RequestUtils.isDispatcherServletRequest()=" + RequestUtils.isDispatcherServletRequest());
-		log.debug("RequestUtils.isZuulServletRequest()=" + RequestUtils.isZuulServletRequest());
-		
+		log.debug("RequestUtils.isDispatcherServletRequest()="
+				+ RequestUtils.isDispatcherServletRequest());
+		log.debug("RequestUtils.isZuulServletRequest()="
+				+ RequestUtils.isZuulServletRequest());
 
 		String adjustedPath = adjustPath(path);
 
@@ -135,7 +136,8 @@ public class SimpleRouteLocator implements RouteLocator {
 			retryable = route.getRetryable();
 		}
 		return new Route(route.getId(), targetPath, route.getLocation(), prefix,
-				retryable, route.getSensitiveHeaders());
+				retryable,
+				route.isCustomSensitiveHeaders() ? route.getSensitiveHeaders() : null);
 	}
 
 	/**

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java

@@ -116,12 +116,11 @@ public class ZuulProperties {
 	private boolean removeSemicolonContent = true;
 
 	/**
-	 * List of sensitive headers that are not passed to downstream requests. Defaults
-	 * to a "safe" set of headers that commonly contain user credentials. It's OK to
-	 * remove those from the list if the downstream service is part of the same system
-	 * as the proxy, so they are sharing authentication data. If using a physical URL
-	 * outside your own domain, then generally it would be a bad idea to leak user
-	 * credentials.
+	 * List of sensitive headers that are not passed to downstream requests. Defaults to a
+	 * "safe" set of headers that commonly contain user credentials. It's OK to remove
+	 * those from the list if the downstream service is part of the same system as the
+	 * proxy, so they are sharing authentication data. If using a physical URL outside
+	 * your own domain, then generally it would be a bad idea to leak user credentials.
 	 */
 	private Set<String> sensitiveHeaders = new LinkedHashSet<>(
 			Arrays.asList("Cookie", "Set-Cookie", "Authorization"));
@@ -158,7 +157,6 @@ public class ZuulProperties {
 	}
 
 	@Data
-	@AllArgsConstructor
 	@NoArgsConstructor
 	public static class ZuulRoute {
 
@@ -206,6 +204,19 @@ public class ZuulProperties {
 		 */
 		private Set<String> sensitiveHeaders = new LinkedHashSet<>();
 
+		private boolean customSensitiveHeaders = false;
+
+		public ZuulRoute(String id, String path, String serviceId, String url,
+				boolean stripPrefix, Boolean retryable, Set<String> sensitiveHeaders) {
+			this.id = id;
+			this.path = path;
+			this.serviceId = serviceId;
+			this.url = url;
+			this.stripPrefix = stripPrefix;
+			this.retryable = retryable;
+			this.sensitiveHeaders = sensitiveHeaders;
+		}
+
 		public ZuulRoute(String text) {
 			String location = null;
 			String path = text;
@@ -254,7 +265,16 @@ public class ZuulProperties {
 
 		public Route getRoute(String prefix) {
 			return new Route(this.id, this.path, getLocation(), prefix, this.retryable,
-					this.sensitiveHeaders);
+					isCustomSensitiveHeaders() ? this.sensitiveHeaders : null);
+		}
+
+		public void setSensitiveHeaders(Set<String> headers) {
+			this.customSensitiveHeaders = true;
+			this.sensitiveHeaders = new LinkedHashSet<>(headers);
+		}
+
+		public boolean isCustomSensitiveHeaders() {
+			return this.customSensitiveHeaders;
 		}
 
 	}

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java

@@ -43,15 +43,15 @@ public class PreDecorationFilter extends ZuulFilter {
 	private ZuulProperties properties;
 
 	private UrlPathHelper urlPathHelper = new UrlPathHelper();
-	
+
 	private ProxyRequestHelper proxyRequestHelper;
 
-	public PreDecorationFilter(RouteLocator routeLocator,
-			String dispatcherServletPath, ZuulProperties properties,
-			ProxyRequestHelper proxyRequestHelper) {
+	public PreDecorationFilter(RouteLocator routeLocator, String dispatcherServletPath,
+			ZuulProperties properties, ProxyRequestHelper proxyRequestHelper) {
 		this.routeLocator = routeLocator;
 		this.properties = properties;
-		this.urlPathHelper.setRemoveSemicolonContent(properties.isRemoveSemicolonContent());
+		this.urlPathHelper
+				.setRemoveSemicolonContent(properties.isRemoveSemicolonContent());
 		this.dispatcherServletPath = dispatcherServletPath;
 		this.proxyRequestHelper = proxyRequestHelper;
 	}
@@ -70,7 +70,8 @@ public class PreDecorationFilter extends ZuulFilter {
 	public boolean shouldFilter() {
 		RequestContext ctx = RequestContext.getCurrentContext();
 		return !ctx.containsKey("forward.to") // a filter has already forwarded
-				&& !ctx.containsKey("serviceId"); // a filter has already determined serviceId
+				&& !ctx.containsKey("serviceId"); // a filter has already determined
+													// serviceId
 	}
 
 	@Override
@@ -84,10 +85,13 @@ public class PreDecorationFilter extends ZuulFilter {
 			if (location != null) {
 				ctx.put("requestURI", route.getPath());
 				ctx.put("proxy", route.getId());
-				if (route.getSensitiveHeaders().isEmpty()) {
-					proxyRequestHelper.addIgnoredHeaders(this.properties.getSensitiveHeaders().toArray(new String[0]));
-				} else {
-					proxyRequestHelper.addIgnoredHeaders(route.getSensitiveHeaders().toArray(new String[0]));
+				if (!route.isCustomSensitiveHeaders()) {
+					this.proxyRequestHelper.addIgnoredHeaders(
+							this.properties.getSensitiveHeaders().toArray(new String[0]));
+				}
+				else {
+					this.proxyRequestHelper.addIgnoredHeaders(
+							route.getSensitiveHeaders().toArray(new String[0]));
 				}
 
 				if (route.getRetryable() != null) {
@@ -118,18 +122,22 @@ public class PreDecorationFilter extends ZuulFilter {
 					ctx.addZuulRequestHeader(ZuulHeaders.X_FORWARDED_PROTO,
 							ctx.getRequest().getScheme());
 					if (StringUtils.hasText(route.getPrefix())) {
-						String existingPrefix = ctx.getRequest().getHeader("X-Forwarded-Prefix");
+						String existingPrefix = ctx.getRequest()
+								.getHeader("X-Forwarded-Prefix");
 						StringBuilder newPrefixBuilder = new StringBuilder();
 						if (StringUtils.hasLength(existingPrefix)) {
-							if (existingPrefix.endsWith("/") && route.getPrefix().startsWith("/")) {
-								newPrefixBuilder.append(existingPrefix, 0, existingPrefix.length() - 1);
+							if (existingPrefix.endsWith("/")
+									&& route.getPrefix().startsWith("/")) {
+								newPrefixBuilder.append(existingPrefix, 0,
+										existingPrefix.length() - 1);
 							}
 							else {
 								newPrefixBuilder.append(existingPrefix);
 							}
 						}
 						newPrefixBuilder.append(route.getPrefix());
-						ctx.addZuulRequestHeader("X-Forwarded-Prefix", newPrefixBuilder.toString());
+						ctx.addZuulRequestHeader("X-Forwarded-Prefix",
+								newPrefixBuilder.toString());
 					}
 					String xforwardedfor = ctx.getRequest().getHeader("X-Forwarded-For");
 					String remoteAddr = ctx.getRequest().getRemoteAddr();
@@ -147,17 +155,21 @@ public class PreDecorationFilter extends ZuulFilter {
 			log.warn("No route found for uri: " + requestURI);
 
 			String fallBackUri = requestURI;
-			String fallbackPrefix = dispatcherServletPath; //default fallback servlet is DispatcherServlet
+			String fallbackPrefix = this.dispatcherServletPath; // default fallback
+																// servlet is
+																// DispatcherServlet
 
 			if (RequestUtils.isZuulServletRequest()) {
-				//remove the Zuul servletPath from the requestUri
+				// remove the Zuul servletPath from the requestUri
 				log.debug("zuulServletPath=" + this.properties.getServletPath());
-				fallBackUri = fallBackUri.replaceFirst(this.properties.getServletPath(), "");
+				fallBackUri = fallBackUri.replaceFirst(this.properties.getServletPath(),
+						"");
 				log.debug("Replaced Zuul servlet path:" + fallBackUri);
-			} else {
-				//remove the DispatcherServlet servletPath from the requestUri
-				log.debug("dispatcherServletPath=" + dispatcherServletPath);
-				fallBackUri = fallBackUri.replaceFirst(dispatcherServletPath, "");
+			}
+			else {
+				// remove the DispatcherServlet servletPath from the requestUri
+				log.debug("dispatcherServletPath=" + this.dispatcherServletPath);
+				fallBackUri = fallBackUri.replaceFirst(this.dispatcherServletPath, "");
 				log.debug("Replaced DispatcherServlet servlet path:" + fallBackUri);
 			}
 			if (!fallBackUri.startsWith("/")) {

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java

@@ -55,7 +55,7 @@ public class PreDecorationFilterTests {
 	private DiscoveryClientRouteLocator routeLocator;
 
 	private MockHttpServletRequest request = new MockHttpServletRequest();
-	
+
 	private ProxyRequestHelper proxyRequestHelper = new ProxyRequestHelper();
 
 	@Before
@@ -64,7 +64,8 @@ public class PreDecorationFilterTests {
 		this.properties = new ZuulProperties();
 		this.routeLocator = new DiscoveryClientRouteLocator("/", this.discovery,
 				this.properties);
-		this.filter = new PreDecorationFilter(this.routeLocator, "/", this.properties, proxyRequestHelper);
+		this.filter = new PreDecorationFilter(this.routeLocator, "/", this.properties,
+				this.proxyRequestHelper);
 		RequestContext ctx = RequestContext.getCurrentContext();
 		ctx.clear();
 		ctx.setRequest(this.request);
@@ -105,7 +106,8 @@ public class PreDecorationFilterTests {
 		assertEquals("80", ctx.getZuulRequestHeaders().get("x-forwarded-port"));
 		assertEquals("http", ctx.getZuulRequestHeaders().get("x-forwarded-proto"));
 		assertEquals("/api", ctx.getZuulRequestHeaders().get("x-forwarded-prefix"));
-		assertEquals("1.2.3.4, 5.6.7.8", ctx.getZuulRequestHeaders().get("x-forwarded-for"));
+		assertEquals("1.2.3.4, 5.6.7.8",
+				ctx.getZuulRequestHeaders().get("x-forwarded-for"));
 		assertEquals("foo",
 				getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid"));
 	}
@@ -126,8 +128,10 @@ public class PreDecorationFilterTests {
 		assertEquals("localhost", ctx.getZuulRequestHeaders().get("x-forwarded-host"));
 		assertEquals("80", ctx.getZuulRequestHeaders().get("x-forwarded-port"));
 		assertEquals("http", ctx.getZuulRequestHeaders().get("x-forwarded-proto"));
-		assertEquals("/prefix/api", ctx.getZuulRequestHeaders().get("x-forwarded-prefix"));
-		assertEquals("1.2.3.4, 5.6.7.8", ctx.getZuulRequestHeaders().get("x-forwarded-for"));
+		assertEquals("/prefix/api",
+				ctx.getZuulRequestHeaders().get("x-forwarded-prefix"));
+		assertEquals("1.2.3.4, 5.6.7.8",
+				ctx.getZuulRequestHeaders().get("x-forwarded-for"));
 		assertEquals("foo",
 				getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid"));
 	}
@@ -169,101 +173,98 @@ public class PreDecorationFilterTests {
 		assertEquals("foo",
 				getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid"));
 	}
-	
+
 	@Test
 	public void routeNotFound() throws Exception {
 		this.properties.setPrefix("/api");
-		this.properties.setStripPrefix(true);		
+		this.properties.setStripPrefix(true);
 		this.routeLocator.addRoute(
 				new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true, null, null));
-		
+
 		this.request.setRequestURI("/api/bar/1");
 
 		this.filter.run();
 		RequestContext ctx = RequestContext.getCurrentContext();
 		assertEquals("/api/bar/1", ctx.get("forward.to"));
 	}
-	
+
 	@Test
 	public void routeNotFoundDispatcherServletSpecialPath() throws Exception {
 		this.properties.setPrefix("/api");
-		this.properties.setStripPrefix(true);	
+		this.properties.setStripPrefix(true);
 		this.properties.setAddProxyHeaders(true);
 		this.routeLocator.addRoute(
 				new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true, null, null));
-		
-		this.filter = new PreDecorationFilter(this.routeLocator,
-				"/special", this.properties, proxyRequestHelper);
-		
+
+		this.filter = new PreDecorationFilter(this.routeLocator, "/special",
+				this.properties, this.proxyRequestHelper);
+
 		this.request.setRequestURI("/api/bar/1");
 
 		this.filter.run();
 		RequestContext ctx = RequestContext.getCurrentContext();
 		assertEquals("/special/api/bar/1", ctx.get("forward.to"));
-	}	
-	
+	}
+
 	@Test
 	public void routeNotFoundZuulRequest() throws Exception {
 		setTestRequestContext();
 		RequestContext ctx = RequestContext.getCurrentContext();
-		ctx.getCurrentContext().setZuulEngineRan();
+		RequestContext.getCurrentContext().setZuulEngineRan();
 		this.request.setRequestURI("/zuul/api/bar/1");
 		ctx.setRequest(this.request);
-		
+
 		this.properties.setPrefix("/api");
 		this.properties.setStripPrefix(true);
 		this.properties.setServletPath("/zuul");
 		this.routeLocator.addRoute(
 				new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true, null, null));
-		
-		
+
 		this.filter.run();
 
 		assertEquals("/api/bar/1", ctx.get("forward.to"));
-	}		
-	
+	}
+
 	@Test
 	public void routeNotFoundZuulRequestDispatcherServletSpecialPath() throws Exception {
 		setTestRequestContext();
 		RequestContext ctx = RequestContext.getCurrentContext();
-		ctx.getCurrentContext().setZuulEngineRan();
+		RequestContext.getCurrentContext().setZuulEngineRan();
 		this.request.setRequestURI("/zuul/api/bar/1");
 		ctx.setRequest(this.request);
-		
+
 		this.properties.setPrefix("/api");
 		this.properties.setStripPrefix(true);
 		this.properties.setServletPath("/zuul");
 		this.properties.setAddProxyHeaders(true);
 		this.routeLocator.addRoute(
 				new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true, null, null));
-		this.filter = new PreDecorationFilter(this.routeLocator,
-				"/special", this.properties, proxyRequestHelper);		
-		
-		
+		this.filter = new PreDecorationFilter(this.routeLocator, "/special",
+				this.properties, this.proxyRequestHelper);
+
 		this.filter.run();
 
 		assertEquals("/special/api/bar/1", ctx.get("forward.to"));
-	}	
-	
-	
+	}
+
 	@Test
 	public void routeNotFoundZuulRequestZuulHomeMapping() throws Exception {
 		setTestRequestContext();
 		RequestContext ctx = RequestContext.getCurrentContext();
-		ctx.getCurrentContext().setZuulEngineRan();
+		RequestContext.getCurrentContext().setZuulEngineRan();
 		this.request.setRequestURI("/api/bar/1");
 		ctx.setRequest(this.request);
-		
+
 		this.properties.setPrefix("/api");
 		this.properties.setStripPrefix(true);
 		this.properties.setServletPath("/");
 		this.properties.setAddProxyHeaders(true);
 		this.routeLocator.addRoute(
 				new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true, null, null));
-		
-		this.filter = new PreDecorationFilter(this.routeLocator,
-				"/special", this.properties, proxyRequestHelper);
-		
+
+		this.filter = new PreDecorationFilter(this.routeLocator, "/special",
+				this.properties, this.proxyRequestHelper);
+
 		this.filter.run();
 
 		assertEquals("/special/api/bar/1", ctx.get("forward.to"));
@@ -281,9 +282,30 @@ public class PreDecorationFilterTests {
 		this.filter.run();
 		RequestContext ctx = RequestContext.getCurrentContext();
 		@SuppressWarnings("unchecked")
-		Set<String> sensitiveHeaders = (Set<String>) ctx.get(ProxyRequestHelper.IGNORED_HEADERS);
-		assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Collections.singletonList("x-foo")));
-		assertFalse("sensitiveHeaders is wrong", sensitiveHeaders.contains("Cookie"));
+		Set<String> sensitiveHeaders = (Set<String>) ctx
+				.get(ProxyRequestHelper.IGNORED_HEADERS);
+		assertTrue("sensitiveHeaders is wrong: " + sensitiveHeaders,
+				sensitiveHeaders.containsAll(Collections.singletonList("x-foo")));
+		assertFalse("sensitiveHeaders is wrong: " + sensitiveHeaders,
+				sensitiveHeaders.contains("Cookie"));
+	}
+
+	@Test
+	public void sensitiveHeadersOverrideEmpty() throws Exception {
+		this.properties.setPrefix("/api");
+		this.properties.setStripPrefix(true);
+		this.properties.setSensitiveHeaders(Collections.singleton("x-bar"));
+		this.request.setRequestURI("/api/foo/1");
+		ZuulRoute route = new ZuulRoute("/foo/**", "foo");
+		route.setSensitiveHeaders(Collections.<String> emptySet());
+		this.routeLocator.addRoute(route);
+		this.filter.run();
+		RequestContext ctx = RequestContext.getCurrentContext();
+		@SuppressWarnings("unchecked")
+		Set<String> sensitiveHeaders = (Set<String>) ctx
+				.get(ProxyRequestHelper.IGNORED_HEADERS);
+		assertTrue("sensitiveHeaders is wrong: " + sensitiveHeaders,
+				sensitiveHeaders.isEmpty());
 	}
 
 	@Test
@@ -296,11 +318,13 @@ public class PreDecorationFilterTests {
 		this.filter.run();
 		RequestContext ctx = RequestContext.getCurrentContext();
 		@SuppressWarnings("unchecked")
-		Set<String> sensitiveHeaders = (Set<String>) ctx.get(ProxyRequestHelper.IGNORED_HEADERS);
-		assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Collections.singletonList("x-bar")));
+		Set<String> sensitiveHeaders = (Set<String>) ctx
+				.get(ProxyRequestHelper.IGNORED_HEADERS);
+		assertTrue("sensitiveHeaders is wrong: " + sensitiveHeaders,
+				sensitiveHeaders.containsAll(Collections.singletonList("x-bar")));
 		assertFalse("sensitiveHeaders is wrong", sensitiveHeaders.contains("Cookie"));
 	}
-	
+
 	@Test
 	public void sensitiveHeadersCaseInsensitive() throws Exception {
 		this.properties.setPrefix("/api");
@@ -311,10 +335,12 @@ public class PreDecorationFilterTests {
 		this.filter.run();
 		RequestContext ctx = RequestContext.getCurrentContext();
 		@SuppressWarnings("unchecked")
-		Set<String> sensitiveHeaders = (Set<String>) ctx.get(ProxyRequestHelper.IGNORED_HEADERS);
-		assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Collections.singletonList("x-bar")));
+		Set<String> sensitiveHeaders = (Set<String>) ctx
+				.get(ProxyRequestHelper.IGNORED_HEADERS);
+		assertTrue("sensitiveHeaders is wrong: " + sensitiveHeaders,
+				sensitiveHeaders.containsAll(Collections.singletonList("x-bar")));
 	}
-	
+
 	@Test
 	public void sensitiveHeadersOverrideCaseInsensitive() throws Exception {
 		this.properties.setPrefix("/api");
@@ -327,23 +353,29 @@ public class PreDecorationFilterTests {
 		this.filter.run();
 		RequestContext ctx = RequestContext.getCurrentContext();
 		@SuppressWarnings("unchecked")
-		Set<String> sensitiveHeaders = (Set<String>) ctx.get(ProxyRequestHelper.IGNORED_HEADERS);
-		assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Collections.singletonList("x-foo")));
+		Set<String> sensitiveHeaders = (Set<String>) ctx
+				.get(ProxyRequestHelper.IGNORED_HEADERS);
+		assertTrue("sensitiveHeaders is wrong: " + sensitiveHeaders,
+				sensitiveHeaders.containsAll(Collections.singletonList("x-foo")));
 	}
-	
+
 	@Test
-	public void ignoredHeadersAlreadySetInRequestContextDontGetOverridden() throws Exception {
+	public void ignoredHeadersAlreadySetInRequestContextDontGetOverridden()
+			throws Exception {
 		this.properties.setPrefix("/api");
 		this.properties.setStripPrefix(true);
 		this.properties.setSensitiveHeaders(Collections.singleton("x-bar"));
 		this.request.setRequestURI("/api/foo/1");
 		this.routeLocator.addRoute("/foo/**", "foo");
 		RequestContext ctx = RequestContext.getCurrentContext();
-		ctx.set(ProxyRequestHelper.IGNORED_HEADERS, new HashSet<>(Arrays.asList("x-foo")));
+		ctx.set(ProxyRequestHelper.IGNORED_HEADERS,
+				new HashSet<>(Arrays.asList("x-foo")));
 		this.filter.run();
 		@SuppressWarnings("unchecked")
-		Set<String> sensitiveHeaders = (Set<String>) ctx.get(ProxyRequestHelper.IGNORED_HEADERS);
-		assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Arrays.asList("x-bar","x-foo")));
+		Set<String> sensitiveHeaders = (Set<String>) ctx
+				.get(ProxyRequestHelper.IGNORED_HEADERS);
+		assertTrue("sensitiveHeaders is wrong: " + sensitiveHeaders,
+				sensitiveHeaders.containsAll(Arrays.asList("x-bar", "x-foo")));
 	}
 
 	private Object getHeader(List<Pair<String, String>> headers, String key) {
@@ -356,11 +388,11 @@ public class PreDecorationFilterTests {
 		}
 		return value;
 	}
-	
+
 	private void setTestRequestContext() {
 		RequestContext context = new RequestContext();
 		RequestContext.testSetCurrentContext(context);
 
-	}	
+	}
 
 }