From 7ebe1977d8a52dd3d738274952d05f2141bdf0ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibaud=20Lepr=C3=AAtre?= Date: Tue, 28 Jun 2016 18:53:25 +0200 Subject: [PATCH] Handle context-path on X-Forwarded-Prefix support Since revision 7b270c4b46c20d42233218082fae0075c3a5c481, Zuul add support of `X-Forwarded-Prefix` for route with `stripPrefix=true`. However it supposes that Zuul context-path is equals to `/`. By using a custom context-path on Zuul we can expect that `zuul.add-proxy-headers=true` will compute a prefix regarding that context-path. Indeed with following architecture: - Zuul with context-path equals to `/foo` - A random service (*bar-service*) without context-path Zuul configurations: ``` zuul: add-proxy-headers: true routes: bar-service: /bar/** ``` We expect that *bar-service* when targeting `http://blabla.com/foo/bar/1` will produce endpoint url equals to `http://blabla.com/foo/bar` and not only `http://blabla.com/bar` (that will not be resolvable). Furthermore context-path influence not only `stripPrefix=true` routes because with following configuration ``` zuul: add-proxy-headers: true routes: bar-service: path: /bar/** strip-prefix: false ``` We expect that *bar-service* when targeting `http://blabla.com/foo/bar/1` will also prodcues endpoint url equals to `http://blabla.com/foo/bar`... --- Moreover since *Spring 4.3* and new [`ForwardedHeaderFilter`](http://docs.spring.io/spring-framework/docs/4.3.0.BUILD-SNAPSHOT/javadoc-api/org/springframework/web/filter/ForwardedHeaderFilter.html) to handle `X-Forwarded-*` headers, when filter catches `X-Forwarded-Prefix` header it will override request context-path by `X-Forwarded-Prefix` header value and then **removes headers from request**. Thus the fact to support context-path will allow to use that filter with Zuul! --- Attention there is only one edge case, when `X-Forwarded-Prefix` header is present as same as custom `context-path`! In that case context-path will be ignored in favor to `X-Forwarded-Prefix` as same does `ForwardedHeaderFilter` --- .../zuul/filters/pre/PreDecorationFilter.java | 24 ++++-- .../filters/pre/PreDecorationFilterTests.java | 82 ++++++++++++++++++- 2 files changed, 96 insertions(+), 10 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java index 39dc918b..d4449f64 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java @@ -121,23 +121,29 @@ public class PreDecorationFilter extends ZuulFilter { String.valueOf(ctx.getRequest().getServerPort())); ctx.addZuulRequestHeader(ZuulHeaders.X_FORWARDED_PROTO, ctx.getRequest().getScheme()); + String forwardedPrefix = + ctx.getRequest().getHeader("X-Forwarded-Prefix"); + String contextPath = ctx.getRequest().getContextPath(); + String prefix = StringUtils.hasLength(forwardedPrefix) + ? forwardedPrefix + : (StringUtils.hasLength(contextPath) ? contextPath : null); if (StringUtils.hasText(route.getPrefix())) { - String existingPrefix = ctx.getRequest() - .getHeader("X-Forwarded-Prefix"); StringBuilder newPrefixBuilder = new StringBuilder(); - if (StringUtils.hasLength(existingPrefix)) { - if (existingPrefix.endsWith("/") + if (prefix != null) { + if (prefix.endsWith("/") && route.getPrefix().startsWith("/")) { - newPrefixBuilder.append(existingPrefix, 0, - existingPrefix.length() - 1); + newPrefixBuilder.append(prefix, 0, + prefix.length() - 1); } else { - newPrefixBuilder.append(existingPrefix); + newPrefixBuilder.append(prefix); } } newPrefixBuilder.append(route.getPrefix()); - ctx.addZuulRequestHeader("X-Forwarded-Prefix", - newPrefixBuilder.toString()); + prefix = newPrefixBuilder.toString(); + } + if (prefix != null) { + ctx.addZuulRequestHeader("X-Forwarded-Prefix", prefix); } String xforwardedfor = ctx.getRequest().getHeader("X-Forwarded-For"); String remoteAddr = ctx.getRequest().getRemoteAddr(); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java index 9b521676..90d65f43 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java @@ -136,6 +136,86 @@ public class PreDecorationFilterTests { getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid")); } + @Test + public void routeWithContextPath() { + this.properties.setStripPrefix(false); + this.request.setRequestURI("/api/foo/1"); + this.request.setContextPath("/context-path"); + this.routeLocator.addRoute( + new ZuulRoute("foo", "/api/foo/**", "foo", null, false, null, null)); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertEquals("/api/foo/1", ctx.get("requestURI")); + 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("/context-path", + ctx.getZuulRequestHeaders().get("x-forwarded-prefix")); + assertEquals("foo", + getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid")); + } + + @Test + public void prefixRouteWithContextPath() { + this.properties.setPrefix("/api"); + this.properties.setStripPrefix(true); + this.request.setRequestURI("/api/foo/1"); + this.request.setContextPath("/context-path"); + this.routeLocator.addRoute( + new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null)); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertEquals("/foo/1", ctx.get("requestURI")); + 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("/context-path/api", + ctx.getZuulRequestHeaders().get("x-forwarded-prefix")); + assertEquals("foo", + getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid")); + } + + @Test + public void routeIgnoreContextPathIfPrefixHeader() { + this.properties.setStripPrefix(false); + this.request.setRequestURI("/api/foo/1"); + this.request.setContextPath("/context-path"); + this.request.addHeader("X-Forwarded-Prefix", "/prefix"); + this.routeLocator.addRoute( + new ZuulRoute("foo", "/api/foo/**", "foo", null, false, null, null)); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertEquals("/api/foo/1", ctx.get("requestURI")); + 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", + ctx.getZuulRequestHeaders().get("x-forwarded-prefix")); + assertEquals("foo", + getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid")); + } + + @Test + public void prefixRouteIgnoreContextPathIfPrefixHeader() { + this.properties.setPrefix("/api"); + this.properties.setStripPrefix(true); + this.request.setRequestURI("/api/foo/1"); + this.request.setContextPath("/context-path"); + this.request.addHeader("X-Forwarded-Prefix", "/prefix"); + this.routeLocator.addRoute( + new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null)); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertEquals("/foo/1", ctx.get("requestURI")); + 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("foo", + getHeader(ctx.getOriginResponseHeaders(), "x-zuul-serviceid")); + } + @Test public void forwardRouteAddsLocation() throws Exception { this.properties.setPrefix("/api"); @@ -377,7 +457,7 @@ public class PreDecorationFilterTests { assertTrue("sensitiveHeaders is wrong: " + sensitiveHeaders, sensitiveHeaders.containsAll(Arrays.asList("x-bar", "x-foo"))); } - + @Test public void urlProperlyDecodedWhenCharacterEncodingIsSet() throws Exception { this.request.setCharacterEncoding("UTF-8");