From 718cafcd0447b00e3ef5a33930a9afdbc2416f40 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 3 Mar 2016 11:29:30 +0000 Subject: [PATCH] Add X-Forwarded-Port explicitly for Tomcat (and other servers) Fixes gh-872 --- .../zuul/filters/pre/PreDecorationFilter.java | 11 ++++-- .../zuul/SampleZuulProxyApplicationTests.java | 37 +++++++++++++------ .../filters/pre/PreDecorationFilterTests.java | 5 ++- 3 files changed, 36 insertions(+), 17 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 695558f8..9642b102 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 @@ -39,7 +39,8 @@ public class PreDecorationFilter extends ZuulFilter { private UrlPathHelper urlPathHelper = new UrlPathHelper(); - public PreDecorationFilter(RouteLocator routeLocator, boolean addProxyHeaders, boolean removeSemicolonContent) { + public PreDecorationFilter(RouteLocator routeLocator, boolean addProxyHeaders, + boolean removeSemicolonContent) { this.routeLocator = routeLocator; this.addProxyHeaders = addProxyHeaders; this.urlPathHelper.setRemoveSemicolonContent(removeSemicolonContent); @@ -59,7 +60,8 @@ public class PreDecorationFilter extends ZuulFilter { public boolean shouldFilter() { RequestContext ctx = RequestContext.getCurrentContext(); return !ctx.containsKey("forward.to") // another filter has already forwarded - && !ctx.containsKey("serviceId"); // another filter has already determined serviceId + && !ctx.containsKey("serviceId"); // another filter has already determined + // serviceId } @Override @@ -97,8 +99,9 @@ public class PreDecorationFilter extends ZuulFilter { } if (this.addProxyHeaders) { ctx.addZuulRequestHeader("X-Forwarded-Host", - ctx.getRequest().getServerName() + ":" - + String.valueOf(ctx.getRequest().getServerPort())); + ctx.getRequest().getServerName()); + ctx.addZuulRequestHeader("X-Forwarded-Port", + String.valueOf(ctx.getRequest().getServerPort())); ctx.addZuulRequestHeader(ZuulHeaders.X_FORWARDED_PROTO, ctx.getRequest().getScheme()); if (StringUtils.hasText(route.getPrefix())) { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyApplicationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyApplicationTests.java index 47a84571..9387f95a 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyApplicationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyApplicationTests.java @@ -62,20 +62,23 @@ import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.ServerList; import com.netflix.niws.client.http.RestClient; -import lombok.SneakyThrows; - +import static org.hamcrest.CoreMatchers.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import lombok.SneakyThrows; + @RunWith(SpringJUnit4ClassRunner.class) @SpringApplicationConfiguration(classes = SampleZuulProxyApplication.class) @WebAppConfiguration -@IntegrationTest({"server.port: 0", +@IntegrationTest({ "server.port: 0", "zuul.routes.other: /test/**=http://localhost:7777/local", "zuul.routes.another: /another/twolevel/**", "zuul.routes.simple: /simple/**", "zuul.routes.badhost: /badhost/**", "zuul.ignoredHeaders: X-Header", - "zuul.removeSemicolonContent: false"}) + "zuul.routes.rnd: /rnd/**", "rnd.ribbon.listOfServers: ${random.value}", + "zuul.removeSemicolonContent: false" }) @DirtiesContext public class SampleZuulProxyApplicationTests extends ZuulProxyTestBase { @@ -140,9 +143,8 @@ public class SampleZuulProxyApplicationTests extends ZuulProxyTestBase { this.routes.addRoute("/self/**", "http://localhost:" + this.port + "/"); this.endpoint.reset(); ResponseEntity result = new TestRestTemplate().exchange( - "http://localhost:" + this.port + "/self/query?foo={foo}", - HttpMethod.GET, new HttpEntity<>((Void) null), String.class, - "weird#chars"); + "http://localhost:" + this.port + "/self/query?foo={foo}", HttpMethod.GET, + new HttpEntity<>((Void) null), String.class, "weird#chars"); assertEquals(HttpStatus.OK, result.getStatusCode()); assertEquals("/query?foo=weird#chars", result.getBody()); } @@ -169,6 +171,18 @@ public class SampleZuulProxyApplicationTests extends ZuulProxyTestBase { "http://localhost:" + this.port + "/badhost/1", HttpMethod.GET, new HttpEntity<>((Void) null), String.class); assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, result.getStatusCode()); + // JSON response + assertThat(result.getBody(), containsString("\"status\":500")); + } + + @Test + public void ribbonCommandRandomHostFromConfig() { + ResponseEntity result = new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/rnd/1", HttpMethod.GET, + new HttpEntity<>((Void) null), String.class); + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, result.getStatusCode()); + // JSON response + assertThat(result.getBody(), containsString("\"status\":500")); } @Test @@ -211,9 +225,10 @@ class SampleZuulProxyApplication extends ZuulProxyTestBase.AbstractZuulProxyAppl @RequestMapping("/matrix/{name}/{another}") public String matrix(@PathVariable("name") String name, - @MatrixVariable(value = "p", pathVar = "name") int p, - @MatrixVariable(value = "q", pathVar = "name") int q, - @PathVariable("another") String another, @MatrixVariable(value = "x", pathVar = "another") int x) { + @MatrixVariable(value = "p", pathVar = "name") int p, + @MatrixVariable(value = "q", pathVar = "name") int q, + @PathVariable("another") String another, + @MatrixVariable(value = "x", pathVar = "another") int x) { return name + "=" + p + "-" + q + ";" + another + "=" + x; } @@ -275,7 +290,7 @@ class SampleZuulProxyApplication extends ZuulProxyTestBase.AbstractZuulProxyAppl } } - // Load balancer with fixed server list for "simple" pointing to localhost + // Load balancer with fixed server list for "simple" pointing to bad host @Configuration static class BadHostRibbonClientConfiguration { @Bean 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 8221c65a..333c6189 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 @@ -90,7 +90,8 @@ public class PreDecorationFilterTests { this.filter.run(); RequestContext ctx = RequestContext.getCurrentContext(); assertEquals("/foo/1", ctx.get("requestURI")); - assertEquals("localhost:80", ctx.getZuulRequestHeaders().get("x-forwarded-host")); + 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("/api", ctx.getZuulRequestHeaders().get("x-forwarded-prefix")); assertEquals("foo", @@ -128,7 +129,7 @@ public class PreDecorationFilterTests { this.filter.run(); RequestContext ctx = RequestContext.getCurrentContext(); assertEquals("/1", ctx.get("requestURI")); - assertEquals("localhost:80", ctx.getZuulRequestHeaders().get("x-forwarded-host")); + assertEquals("localhost", ctx.getZuulRequestHeaders().get("x-forwarded-host")); assertEquals("http", ctx.getZuulRequestHeaders().get("x-forwarded-proto")); assertEquals("/api/foo", ctx.getZuulRequestHeaders().get("x-forwarded-prefix")); assertEquals("foo",