Commit 2f905b81 by Adrian Ivan Committed by Dave Syer

Detect Zuul vs DispatcherServlet execution

Change SimpleRouteLocator to work with both Zuul and DispatcherServlet Fixes gh-794
parent 1534ca41
...@@ -36,6 +36,7 @@ import org.springframework.cloud.netflix.zuul.filters.discovery.DiscoveryClientR ...@@ -36,6 +36,7 @@ import org.springframework.cloud.netflix.zuul.filters.discovery.DiscoveryClientR
import org.springframework.cloud.netflix.zuul.filters.discovery.ServiceRouteMapper; import org.springframework.cloud.netflix.zuul.filters.discovery.ServiceRouteMapper;
import org.springframework.cloud.netflix.zuul.filters.discovery.SimpleServiceRouteMapper; import org.springframework.cloud.netflix.zuul.filters.discovery.SimpleServiceRouteMapper;
import org.springframework.cloud.netflix.zuul.filters.pre.PreDecorationFilter; import org.springframework.cloud.netflix.zuul.filters.pre.PreDecorationFilter;
import org.springframework.cloud.netflix.zuul.filters.pre.ServletDetectionFilter;
import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonCommandFactory;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory;
import org.springframework.cloud.netflix.zuul.filters.route.RibbonRoutingFilter; import org.springframework.cloud.netflix.zuul.filters.route.RibbonRoutingFilter;
...@@ -92,6 +93,11 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { ...@@ -92,6 +93,11 @@ public class ZuulProxyConfiguration extends ZuulConfiguration {
// pre filters // pre filters
@Bean @Bean
public ServletDetectionFilter servletDetectionFilter() {
return new ServletDetectionFilter();
}
@Bean
public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator) { public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator) {
return new PreDecorationFilter(routeLocator, return new PreDecorationFilter(routeLocator,
this.zuulProperties.isAddProxyHeaders()); this.zuulProperties.isAddProxyHeaders());
......
...@@ -24,12 +24,15 @@ import java.util.Map; ...@@ -24,12 +24,15 @@ import java.util.Map;
import java.util.Map.Entry; import java.util.Map.Entry;
import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.atomic.AtomicReference;
import lombok.extern.apachecommons.CommonsLog;
import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute;
import org.springframework.cloud.netflix.zuul.util.RequestUtils;
import org.springframework.util.AntPathMatcher; import org.springframework.util.AntPathMatcher;
import org.springframework.util.PathMatcher; import org.springframework.util.PathMatcher;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
import lombok.extern.apachecommons.CommonsLog; import com.netflix.zuul.http.ZuulServlet;
/** /**
* Simple {@link RouteLocator} based on configuration data held in {@link ZuulProperties}. * Simple {@link RouteLocator} based on configuration data held in {@link ZuulProperties}.
...@@ -43,20 +46,18 @@ public class SimpleRouteLocator implements RouteLocator { ...@@ -43,20 +46,18 @@ public class SimpleRouteLocator implements RouteLocator {
private PathMatcher pathMatcher = new AntPathMatcher(); private PathMatcher pathMatcher = new AntPathMatcher();
private String servletPath; private String dispatcherServletPath = "/";
private String zuulServletPath;
private AtomicReference<Map<String, ZuulRoute>> routes = new AtomicReference<>(); private AtomicReference<Map<String, ZuulRoute>> routes = new AtomicReference<>();
public SimpleRouteLocator(String servletPath, ZuulProperties properties) { public SimpleRouteLocator(String servletPath, ZuulProperties properties) {
this.properties = properties; this.properties = properties;
if (StringUtils.hasText(servletPath)) { // a servletPath is passed explicitly if (servletPath != null && StringUtils.hasText(servletPath)) {
this.servletPath = servletPath; this.dispatcherServletPath = servletPath;
}
else {
// set Zuul servlet path
this.servletPath = properties.getServletPath() != null
? properties.getServletPath() : "";
} }
this.zuulServletPath = properties.getServletPath();
} }
@Override @Override
...@@ -79,7 +80,7 @@ public class SimpleRouteLocator implements RouteLocator { ...@@ -79,7 +80,7 @@ public class SimpleRouteLocator implements RouteLocator {
} }
@Override @Override
public Route getMatchingRoute(String path) { public Route getMatchingRoute(final String path) {
if (log.isDebugEnabled()) { if (log.isDebugEnabled()) {
log.debug("Finding route for path: " + path); log.debug("Finding route for path: " + path);
...@@ -89,25 +90,25 @@ public class SimpleRouteLocator implements RouteLocator { ...@@ -89,25 +90,25 @@ public class SimpleRouteLocator implements RouteLocator {
this.routes.set(locateRoutes()); this.routes.set(locateRoutes());
} }
log.debug("servletPath=" + this.servletPath); log.debug("servletPath=" + this.dispatcherServletPath);
if (StringUtils.hasText(this.servletPath) && !this.servletPath.equals("/") log.debug("zuulServletPath=" + this.zuulServletPath);
&& path.startsWith(this.servletPath)) {
path = path.substring(this.servletPath.length()); String adjustedPath = adjustPath(path);
}
log.debug("path=" + path);
ZuulRoute route = null; ZuulRoute route = null;
if (!matchesIgnoredPatterns(path)) { if (!matchesIgnoredPatterns(adjustedPath)) {
for (Entry<String, ZuulRoute> entry : this.routes.get().entrySet()) { for (Entry<String, ZuulRoute> entry : this.routes.get().entrySet()) {
String pattern = entry.getKey(); String pattern = entry.getKey();
log.debug("Matching pattern:" + pattern); log.debug("Matching pattern:" + pattern);
if (this.pathMatcher.match(pattern, path)) { if (this.pathMatcher.match(pattern, adjustedPath)) {
route = entry.getValue(); route = entry.getValue();
break; break;
} }
} }
} }
log.debug("route matched=" + route);
return getRoute(route, path); return getRoute(route, adjustedPath);
} }
...@@ -167,4 +168,26 @@ public class SimpleRouteLocator implements RouteLocator { ...@@ -167,4 +168,26 @@ public class SimpleRouteLocator implements RouteLocator {
return false; return false;
} }
private String adjustPath(final String path) {
String adjustedPath = path;
if (RequestUtils.isDispatcherServletRequest()
&& StringUtils.hasText(dispatcherServletPath)) {
if (!dispatcherServletPath.equals("/")) {
adjustedPath = path.substring(this.dispatcherServletPath.length());
}
} else if (RequestUtils.isZuulServletRequest()){
if (StringUtils.hasText(zuulServletPath)
&& !zuulServletPath.equals("/")) {
adjustedPath = path.substring(this.zuulServletPath.length());
}
} else {
//do nothing
}
log.debug("adjustedPath=" + path);
return adjustedPath;
}
} }
...@@ -20,9 +20,9 @@ import java.lang.reflect.Field; ...@@ -20,9 +20,9 @@ import java.lang.reflect.Field;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import org.springframework.cloud.netflix.zuul.util.RequestUtils;
import org.springframework.util.Assert; import org.springframework.util.Assert;
import org.springframework.util.ReflectionUtils; import org.springframework.util.ReflectionUtils;
import org.springframework.web.servlet.DispatcherServlet;
import com.netflix.zuul.ZuulFilter; import com.netflix.zuul.ZuulFilter;
import com.netflix.zuul.context.RequestContext; import com.netflix.zuul.context.RequestContext;
...@@ -71,16 +71,11 @@ public class Servlet30WrapperFilter extends ZuulFilter { ...@@ -71,16 +71,11 @@ public class Servlet30WrapperFilter extends ZuulFilter {
request); request);
ctx.setRequest(new Servlet30RequestWrapper(request)); ctx.setRequest(new Servlet30RequestWrapper(request));
} }
else if (isDispatcherServletRequest(request)) { else if (RequestUtils.isDispatcherServletRequest()) {
// If it's going through the dispatcher we need to buffer the body // If it's going through the dispatcher we need to buffer the body
ctx.setRequest(new Servlet30RequestWrapper(request)); ctx.setRequest(new Servlet30RequestWrapper(request));
} }
return null; return null;
} }
private boolean isDispatcherServletRequest(HttpServletRequest request) {
return request.getAttribute(
DispatcherServlet.WEB_APPLICATION_CONTEXT_ATTRIBUTE) != null;
}
} }
/*
* Copyright 2013-2015 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.cloud.netflix.zuul.filters.pre;
import java.lang.reflect.Field;
import javax.servlet.http.HttpServletRequest;
import org.springframework.cloud.netflix.zuul.util.RequestUtils;
import org.springframework.web.servlet.DispatcherServlet;
import com.netflix.zuul.ZuulFilter;
import com.netflix.zuul.context.RequestContext;
import com.netflix.zuul.http.HttpServletRequestWrapper;
import com.netflix.zuul.http.ZuulServlet;
/**
* Detects whether a request is ran through the {@link DispatcherServlet} or {@link ZuulServlet}.
* The purpose was to detect this up-front at the very beginning of Zuul filter processing
* and rely on this information in all filters.
* RequestContext is used such that the information is accessible to classes
* which do not have a request reference.
* @author Adrian Ivan
*/
public class ServletDetectionFilter extends ZuulFilter {
public ServletDetectionFilter() {
}
@Override
public String filterType() {
return "pre";
}
/**
* Must run before other filters that rely on the difference between
* DispatcherServlet and ZuulServlet.
*/
@Override
public int filterOrder() {
return -3;
}
@Override
public boolean shouldFilter() {
return true;
}
@Override
public Object run() {
RequestContext ctx = RequestContext.getCurrentContext();
HttpServletRequest request = ctx.getRequest();
if (!(request instanceof HttpServletRequestWrapper)
&& isDispatcherServletRequest(request)) {
ctx.set(RequestUtils.IS_DISPATCHERSERVLETREQUEST, true);
} else {
ctx.set(RequestUtils.IS_DISPATCHERSERVLETREQUEST, false);
}
return null;
}
private boolean isDispatcherServletRequest(HttpServletRequest request) {
return request.getAttribute(DispatcherServlet.WEB_APPLICATION_CONTEXT_ATTRIBUTE) != null;
}
}
package org.springframework.cloud.netflix.zuul.util;
import com.netflix.zuul.context.RequestContext;
public class RequestUtils {
public static final String IS_DISPATCHERSERVLETREQUEST = "isDispatcherServletRequest";
public static boolean isDispatcherServletRequest() {
return RequestContext.getCurrentContext().getBoolean(IS_DISPATCHERSERVLETREQUEST);
}
public static boolean isZuulServletRequest() {
//extra check for dispatcher since ZuulServlet can run from ZuulController
return !isDispatcherServletRequest() && RequestContext.getCurrentContext().getZuulEngineRan();
}
}
...@@ -28,8 +28,11 @@ import org.springframework.cloud.client.discovery.DiscoveryClient; ...@@ -28,8 +28,11 @@ import org.springframework.cloud.client.discovery.DiscoveryClient;
import org.springframework.cloud.netflix.zuul.filters.Route; import org.springframework.cloud.netflix.zuul.filters.Route;
import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties;
import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute;
import org.springframework.cloud.netflix.zuul.util.RequestUtils;
import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.ConfigurableEnvironment;
import com.netflix.zuul.context.RequestContext;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
...@@ -63,6 +66,7 @@ public class DiscoveryClientRouteLocatorTests { ...@@ -63,6 +66,7 @@ public class DiscoveryClientRouteLocatorTests {
@Before @Before
public void init() { public void init() {
initMocks(this); initMocks(this);
setTestRequestcontext(); //re-initialize Zuul context for each test
} }
@Test @Test
...@@ -92,6 +96,8 @@ public class DiscoveryClientRouteLocatorTests { ...@@ -92,6 +96,8 @@ public class DiscoveryClientRouteLocatorTests {
@Test @Test
public void testGetMatchingPathWithServletPath() throws Exception { public void testGetMatchingPathWithServletPath() throws Exception {
setTestRequestcontext();
RequestContext.getCurrentContext().set(RequestUtils.IS_DISPATCHERSERVLETREQUEST, true);
DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/app", DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/app",
this.discovery, this.properties); this.discovery, this.properties);
this.properties.getRoutes().put("foo", new ZuulRoute("/foo/**")); this.properties.getRoutes().put("foo", new ZuulRoute("/foo/**"));
...@@ -103,6 +109,20 @@ public class DiscoveryClientRouteLocatorTests { ...@@ -103,6 +109,20 @@ public class DiscoveryClientRouteLocatorTests {
} }
@Test @Test
public void testGetMatchingPathWithZuulServletPath() throws Exception {
RequestContext.getCurrentContext().setZuulEngineRan();
DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/app",
this.discovery, this.properties);
this.properties.getRoutes().put("foo", new ZuulRoute("/foo/**"));
this.properties.init();
routeLocator.getRoutes(); // force refresh
Route route = routeLocator.getMatchingRoute("/zuul/foo/1");
assertEquals("foo", route.getLocation());
assertEquals("/1", route.getPath());
}
@Test
public void testGetMatchingPathWithNoPrefixStripping() throws Exception { public void testGetMatchingPathWithNoPrefixStripping() throws Exception {
DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/", DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/",
this.discovery, this.properties); this.discovery, this.properties);
...@@ -143,6 +163,34 @@ public class DiscoveryClientRouteLocatorTests { ...@@ -143,6 +163,34 @@ public class DiscoveryClientRouteLocatorTests {
} }
@Test @Test
public void testGetMatchingPathWithGlobalPrefixStrippingAndServletPath() throws Exception {
RequestContext.getCurrentContext().set(RequestUtils.IS_DISPATCHERSERVLETREQUEST, true);
DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/app",
this.discovery, this.properties);
this.properties.getRoutes().put("foo",
new ZuulRoute("foo", "/foo/**", "foo", null, false, null));
this.properties.setPrefix("/proxy");
routeLocator.getRoutes(); // force refresh
Route route = routeLocator.getMatchingRoute("/app/proxy/foo/1");
assertEquals("foo", route.getLocation());
assertEquals("/foo/1", route.getPath());
}
@Test
public void testGetMatchingPathWithGlobalPrefixStrippingAndZuulServletPath() throws Exception {
RequestContext.getCurrentContext().setZuulEngineRan();
DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/",
this.discovery, this.properties);
this.properties.getRoutes().put("foo",
new ZuulRoute("foo", "/foo/**", "foo", null, false, null));
this.properties.setPrefix("/proxy");
routeLocator.getRoutes(); // force refresh
Route route = routeLocator.getMatchingRoute("/zuul/proxy/foo/1");
assertEquals("foo", route.getLocation());
assertEquals("/foo/1", route.getPath());
}
@Test
public void testGetMatchingPathWithRoutePrefixStripping() throws Exception { public void testGetMatchingPathWithRoutePrefixStripping() throws Exception {
DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/", DiscoveryClientRouteLocator routeLocator = new DiscoveryClientRouteLocator("/",
this.discovery, this.properties); this.discovery, this.properties);
...@@ -596,4 +644,10 @@ public class DiscoveryClientRouteLocatorTests { ...@@ -596,4 +644,10 @@ public class DiscoveryClientRouteLocatorTests {
} }
return null; return null;
} }
private void setTestRequestcontext() {
RequestContext context = new RequestContext();
RequestContext.testSetCurrentContext(context);
}
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment