Sync up matching of ignored paths and route paths (#2163)

Signed-off-by: João Salavessa <joao.salavessa@sky.uk>

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2015 the original author or authors.
+ * Copyright 2013-2017 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.
@@ -24,7 +24,8 @@ import org.springframework.boot.autoconfigure.web.ErrorController;
 import org.springframework.cloud.netflix.zuul.filters.RefreshableRouteLocator;
 import org.springframework.cloud.netflix.zuul.filters.Route;
 import org.springframework.cloud.netflix.zuul.filters.RouteLocator;
-import org.springframework.util.PatternMatchUtils;
+import org.springframework.util.AntPathMatcher;
+import org.springframework.util.PathMatcher;
 import org.springframework.web.cors.CorsConfiguration;
 import org.springframework.web.servlet.HandlerExecutionChain;
 import org.springframework.web.servlet.handler.AbstractUrlHandlerMapping;
@@ -36,6 +37,8 @@ import com.netflix.zuul.context.RequestContext;
  *
  * @author Spencer Gibb
  * @author Dave Syer
+ * @author João Salavessa
+ * @author Biju Kunjummen
  */
 public class ZuulHandlerMapping extends AbstractUrlHandlerMapping {
 
@@ -45,6 +48,8 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping {
 
 	private ErrorController errorController;
 
+	private PathMatcher pathMatcher = new AntPathMatcher();
+
 	private volatile boolean dirty = true;
 
 	public ZuulHandlerMapping(RouteLocator routeLocator, ZuulController zuul) {
@@ -79,10 +84,7 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping {
 		if (this.errorController != null && urlPath.equals(this.errorController.getErrorPath())) {
 			return null;
 		}
-		String[] ignored = this.routeLocator.getIgnoredPaths().toArray(new String[0]);
-		if (PatternMatchUtils.simpleMatch(ignored, urlPath)) {
-			return null;
-		}
+		if (isIgnoredPath(urlPath, this.routeLocator.getIgnoredPaths())) return null;
 		RequestContext ctx = RequestContext.getCurrentContext();
 		if (ctx.containsKey("forward.to")) {
 			return null;
@@ -98,6 +100,17 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping {
 		return super.lookupHandler(urlPath, request);
 	}
 
+	private boolean isIgnoredPath(String urlPath, Collection<String> ignored) {
+		if (ignored != null) {
+			for (String ignoredPath : ignored) {
+				if (this.pathMatcher.match(ignoredPath, urlPath)) {
+					return true;
+				}
+			}
+		}
+		return false;
+	}
+
 	private void registerHandlers() {
 		Collection<Route> routes = this.routeLocator.getRoutes();
 		if (routes.isEmpty()) {

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMappingTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2015 the original author or authors.
+ * Copyright 2013-2017 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.
@@ -16,7 +16,11 @@
 
 package org.springframework.cloud.netflix.zuul.web;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -28,11 +32,9 @@ import org.springframework.mock.web.MockHttpServletRequest;
 
 import com.netflix.zuul.context.RequestContext;
 
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-
 /**
  * @author Dave Syer
+ * @author Biju Kunjummen
  */
 public class ZuulHandlerMappingTests {
 
@@ -58,7 +60,7 @@ public class ZuulHandlerMappingTests {
 				.singletonList(new Route("foo", "/foo/**", "foo", "", null, null)));
 		this.request.setServletPath("/foo/");
 		this.mapping.setDirty(true);
-		assertNotNull(this.mapping.getHandler(this.request));
+		assertThat(this.mapping.getHandler(this.request)).isNotNull();
 	}
 
 	@Test
@@ -68,7 +70,7 @@ public class ZuulHandlerMappingTests {
 		;
 		this.request.setServletPath("/");
 		this.mapping.setDirty(true);
-		assertNotNull(this.mapping.getHandler(this.request));
+		assertThat(this.mapping.getHandler(this.request)).isNotNull();
 	}
 
 	@Test
@@ -77,7 +79,41 @@ public class ZuulHandlerMappingTests {
 				.singletonList(new Route("default", "/**", "foo", "", null, null)));
 		this.request.setServletPath("/error");
 		this.mapping.setDirty(true);
-		assertNull(this.mapping.getHandler(this.request));
+		assertThat(this.mapping.getHandler(this.request)).isNull();
+	}
+
+	@Test
+	public void ignoredPathsShouldNotReturnAHandler() throws Exception {
+		assertThat(mappingWithIgnoredPathsAndRoutes(Arrays.asList("/p1/**"),
+				new Route("p1", "/p1/**", "p1", "", null, null))
+						.getHandler(requestForAPath("/p1"))).isNull();
+
+		assertThat(mappingWithIgnoredPathsAndRoutes(Arrays.asList("/p1/**/p3/"),
+				new Route("p1", "/p1/**/p3", "p1", "", null, null))
+				.getHandler(requestForAPath("/p1/p2/p3"))).isNull();
+
+		assertThat(mappingWithIgnoredPathsAndRoutes(Arrays.asList("/p1/**/p3/**"),
+				new Route("p1", "/p1/**/p3", "p1", "", null, null))
+				.getHandler(requestForAPath("/p1/p2/p3"))).isNull();
+
+		assertThat(mappingWithIgnoredPathsAndRoutes(Arrays.asList("/p1/**/p4/"),
+				new Route("p1", "/p1/**/p4/", "p1", "", null, null))
+				.getHandler(requestForAPath("/p1/p2/p3/p4"))).isNull();
+	}
+	
+	private ZuulHandlerMapping mappingWithIgnoredPathsAndRoutes(List<String> ignoredPaths, Route route) {
+		RouteLocator routeLocator = Mockito.mock(RouteLocator.class);
+		Mockito.when(routeLocator.getIgnoredPaths())
+				.thenReturn(ignoredPaths);
+		Mockito.when(routeLocator.getRoutes()).thenReturn(Collections.singletonList(route));
+		ZuulHandlerMapping zuulHandlerMapping = new ZuulHandlerMapping(routeLocator, new ZuulController());
+		return zuulHandlerMapping;
+	}
+	
+	private MockHttpServletRequest requestForAPath(String path) {
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.setServletPath(path);
+		return request;
 	}
 
 }