From 0e1b856fc5e6aedd8dc6ccb9ea4dc693d89794ac Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 25 Oct 2016 07:50:32 +0100 Subject: [PATCH 1/5] Extract proxy header manipulation into method --- .../zuul/filters/pre/PreDecorationFilter.java | 93 ++++++++++--------- 1 file changed, 48 insertions(+), 45 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 6db078a9..1b5bd474 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 @@ -50,12 +50,11 @@ public class PreDecorationFilter extends ZuulFilter { 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; } @@ -81,8 +80,7 @@ public class PreDecorationFilter extends ZuulFilter { @Override public Object run() { RequestContext ctx = RequestContext.getCurrentContext(); - final String requestURI = this.urlPathHelper - .getPathWithinApplication(ctx.getRequest()); + final String requestURI = this.urlPathHelper.getPathWithinApplication(ctx.getRequest()); Route route = this.routeLocator.getMatchingRoute(requestURI); if (route != null) { String location = route.getLocation(); @@ -90,12 +88,11 @@ public class PreDecorationFilter extends ZuulFilter { ctx.put("requestURI", route.getPath()); ctx.put("proxy", route.getId()); if (!route.isCustomSensitiveHeaders()) { - this.proxyRequestHelper.addIgnoredHeaders( - this.properties.getSensitiveHeaders().toArray(new String[0])); + this.proxyRequestHelper + .addIgnoredHeaders(this.properties.getSensitiveHeaders().toArray(new String[0])); } else { - this.proxyRequestHelper.addIgnoredHeaders( - route.getSensitiveHeaders().toArray(new String[0])); + this.proxyRequestHelper.addIgnoredHeaders(route.getSensitiveHeaders().toArray(new String[0])); } if (route.getRetryable() != null) { @@ -107,8 +104,8 @@ public class PreDecorationFilter extends ZuulFilter { ctx.addOriginResponseHeader("X-Zuul-Service", location); } else if (location.startsWith("forward:")) { - ctx.set("forward.to", StringUtils.cleanPath( - location.substring("forward:".length()) + route.getPath())); + ctx.set("forward.to", + StringUtils.cleanPath(location.substring("forward:".length()) + route.getPath())); ctx.setRouteHost(null); return null; } @@ -119,35 +116,7 @@ public class PreDecorationFilter extends ZuulFilter { ctx.addOriginResponseHeader("X-Zuul-ServiceId", location); } if (this.properties.isAddProxyHeaders()) { - ctx.addZuulRequestHeader("X-Forwarded-Host", toHostHeader(ctx.getRequest())); - ctx.addZuulRequestHeader("X-Forwarded-Port", - 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())) { - StringBuilder newPrefixBuilder = new StringBuilder(); - if (prefix != null) { - if (prefix.endsWith("/") - && route.getPrefix().startsWith("/")) { - newPrefixBuilder.append(prefix, 0, - prefix.length() - 1); - } - else { - newPrefixBuilder.append(prefix); - } - } - newPrefixBuilder.append(route.getPrefix()); - prefix = newPrefixBuilder.toString(); - } - if (prefix != null) { - ctx.addZuulRequestHeader("X-Forwarded-Prefix", prefix); - } + addProxyHeaders(ctx, route); String xforwardedfor = ctx.getRequest().getHeader("X-Forwarded-For"); String remoteAddr = ctx.getRequest().getRemoteAddr(); if (xforwardedfor == null) { @@ -174,8 +143,7 @@ public class PreDecorationFilter extends ZuulFilter { if (RequestUtils.isZuulServletRequest()) { // 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 { @@ -194,11 +162,46 @@ public class PreDecorationFilter extends ZuulFilter { return null; } + private void addProxyHeaders(RequestContext ctx, Route route) { + String host = toHostHeader(ctx.getRequest()); + String port = String.valueOf(ctx.getRequest().getServerPort()); + String proto = ctx.getRequest().getScheme(); + ctx.addZuulRequestHeader("X-Forwarded-Host", host); + ctx.addZuulRequestHeader("X-Forwarded-Port", port); + ctx.addZuulRequestHeader(ZuulHeaders.X_FORWARDED_PROTO, proto); + addProxyPrefix(ctx, route); + } + + private void addProxyPrefix(RequestContext ctx, Route route) { + 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())) { + StringBuilder newPrefixBuilder = new StringBuilder(); + if (prefix != null) { + if (prefix.endsWith("/") && route.getPrefix().startsWith("/")) { + newPrefixBuilder.append(prefix, 0, prefix.length() - 1); + } + else { + newPrefixBuilder.append(prefix); + } + } + newPrefixBuilder.append(route.getPrefix()); + prefix = newPrefixBuilder.toString(); + } + if (prefix != null) { + ctx.addZuulRequestHeader("X-Forwarded-Prefix", prefix); + } + } + private String toHostHeader(HttpServletRequest request) { int port = request.getServerPort(); - if ((port == 80 && "http".equals(request.getScheme())) || (port == 443 && "https".equals(request.getScheme()))) { + if ((port == 80 && "http".equals(request.getScheme())) + || (port == 443 && "https".equals(request.getScheme()))) { return request.getServerName(); - } else { + } + else { return request.getServerName() + ":" + port; } } From 71ac67f9f17938277afe78482923695af5a60134 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 25 Oct 2016 08:50:26 +0100 Subject: [PATCH 2/5] Suppress some warnings --- .../cloud/netflix/zuul/ZuulProxyConfiguration.java | 1 + .../cloud/netflix/AdhocTestSuite.java | 12 +++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java index fa3e2838..11ee38b1 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java @@ -70,6 +70,7 @@ import org.springframework.context.annotation.Configuration; @Configuration public class ZuulProxyConfiguration extends ZuulConfiguration { + @SuppressWarnings("rawtypes") @Autowired(required = false) private List requestCustomizers = Collections.emptyList(); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/AdhocTestSuite.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/AdhocTestSuite.java index 2200743e..47a365ed 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/AdhocTestSuite.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/AdhocTestSuite.java @@ -20,7 +20,11 @@ import org.junit.Ignore; import org.junit.runner.RunWith; import org.junit.runners.Suite; import org.junit.runners.Suite.SuiteClasses; -import org.springframework.cloud.netflix.zuul.filters.route.restclient.RestClientRibbonCommandIntegrationTests; +import org.springframework.cloud.netflix.feign.encoding.FeignAcceptEncodingTests; +import org.springframework.cloud.netflix.metrics.servo.ServoMetricReaderTests; +import org.springframework.cloud.netflix.ribbon.RibbonInterceptorTests; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClientTests; +import org.springframework.cloud.netflix.zuul.ZuulProxyConfigurationTests; /** * A test suite for probing weird ordering problems in the tests. @@ -28,10 +32,8 @@ import org.springframework.cloud.netflix.zuul.filters.route.restclient.RestClien * @author Dave Syer */ @RunWith(Suite.class) -@SuiteClasses({ - org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelperTests.class, - RestClientRibbonCommandIntegrationTests.class, - org.springframework.cloud.netflix.zuul.FormZuulProxyApplicationTests.class }) +@SuiteClasses({ RibbonLoadBalancerClientTests.class, RibbonInterceptorTests.class, FeignAcceptEncodingTests.class, + ServoMetricReaderTests.class, ZuulProxyConfigurationTests.class }) @Ignore public class AdhocTestSuite { From 139ab95438d0eea0c90c8aea23f3098caeca2333 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 25 Oct 2016 09:35:35 +0100 Subject: [PATCH 3/5] Make configuration imports deterministic --- .../RibbonCommandFactoryConfiguration.java | 154 ++++++++++++++++++ .../netflix/zuul/ZuulProxyConfiguration.java | 143 ++-------------- 2 files changed, 166 insertions(+), 131 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java new file mode 100644 index 00000000..95741603 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java @@ -0,0 +1,154 @@ +/* + * Copyright 2015-2016 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; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.Collections; +import java.util.Set; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.cloud.netflix.ribbon.SpringClientFactory; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; +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.ZuulFallbackProvider; +import org.springframework.cloud.netflix.zuul.filters.route.apache.HttpClientRibbonCommandFactory; +import org.springframework.cloud.netflix.zuul.filters.route.okhttp.OkHttpRibbonCommandFactory; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; +import org.springframework.context.annotation.Configuration; + +/** + * @author Dave Syer + * + */ +public class RibbonCommandFactoryConfiguration { + + @Configuration + @ConditionalOnRibbonRestClient + protected static class RestClientRibbonConfiguration { + + @Autowired(required = false) + private Set zuulFallbackProviders = Collections.emptySet(); + + @Bean + @ConditionalOnMissingBean + public RibbonCommandFactory ribbonCommandFactory( + SpringClientFactory clientFactory, ZuulProperties zuulProperties) { + return new RestClientRibbonCommandFactory(clientFactory, zuulProperties, + zuulFallbackProviders); + } + } + + @Configuration + @ConditionalOnRibbonOkHttpClient + @ConditionalOnClass(name = "okhttp3.OkHttpClient") + protected static class OkHttpRibbonConfiguration { + + @Autowired(required = false) + private Set zuulFallbackProviders = Collections.emptySet(); + + @Bean + @ConditionalOnMissingBean + public RibbonCommandFactory ribbonCommandFactory( + SpringClientFactory clientFactory, ZuulProperties zuulProperties) { + return new OkHttpRibbonCommandFactory(clientFactory, zuulProperties, + zuulFallbackProviders); + } + } + + @Configuration + @ConditionalOnRibbonHttpClient + protected static class HttpClientRibbonConfiguration { + + @Autowired(required = false) + private Set zuulFallbackProviders = Collections.emptySet(); + + @Bean + @ConditionalOnMissingBean + public RibbonCommandFactory ribbonCommandFactory( + SpringClientFactory clientFactory, ZuulProperties zuulProperties) { + return new HttpClientRibbonCommandFactory(clientFactory, zuulProperties, zuulFallbackProviders); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @Conditional(OnRibbonHttpClientCondition.class) + @interface ConditionalOnRibbonHttpClient { } + + private static class OnRibbonHttpClientCondition extends AnyNestedCondition { + public OnRibbonHttpClientCondition() { + super(ConfigurationPhase.PARSE_CONFIGURATION); + } + + @Deprecated //remove in Edgware" + @ConditionalOnProperty(name = "zuul.ribbon.httpclient.enabled", matchIfMissing = true) + static class ZuulProperty {} + + @ConditionalOnProperty(name = "ribbon.httpclient.enabled", matchIfMissing = true) + static class RibbonProperty {} + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @Conditional(OnRibbonOkHttpClientCondition.class) + @interface ConditionalOnRibbonOkHttpClient { } + + private static class OnRibbonOkHttpClientCondition extends AnyNestedCondition { + public OnRibbonOkHttpClientCondition() { + super(ConfigurationPhase.PARSE_CONFIGURATION); + } + + @Deprecated //remove in Edgware" + @ConditionalOnProperty("zuul.ribbon.okhttp.enabled") + static class ZuulProperty {} + + @ConditionalOnProperty("ribbon.okhttp.enabled") + static class RibbonProperty {} + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @Conditional(OnRibbonRestClientCondition.class) + @interface ConditionalOnRibbonRestClient { } + + private static class OnRibbonRestClientCondition extends AnyNestedCondition { + public OnRibbonRestClientCondition() { + super(ConfigurationPhase.PARSE_CONFIGURATION); + } + + @Deprecated //remove in Edgware" + @ConditionalOnProperty("zuul.ribbon.restclient.enabled") + static class ZuulProperty {} + + @ConditionalOnProperty("ribbon.restclient.enabled") + static class RibbonProperty {} + } + +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java index 11ee38b1..40ea7a9c 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java @@ -16,30 +16,21 @@ package org.springframework.cloud.netflix.zuul; -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; import java.util.Collections; import java.util.List; -import java.util.Set; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.trace.TraceRepository; -import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.client.actuator.HasFeatures; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.client.discovery.event.HeartbeatEvent; import org.springframework.cloud.client.discovery.event.HeartbeatMonitor; import org.springframework.cloud.client.discovery.event.InstanceRegisteredEvent; import org.springframework.cloud.client.discovery.event.ParentHeartbeatEvent; -import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.RouteLocator; @@ -49,25 +40,24 @@ 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.SimpleServiceRouteMapper; import org.springframework.cloud.netflix.zuul.filters.pre.PreDecorationFilter; -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.RibbonRoutingFilter; import org.springframework.cloud.netflix.zuul.filters.route.SimpleHostRoutingFilter; -import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; -import org.springframework.cloud.netflix.zuul.filters.route.apache.HttpClientRibbonCommandFactory; -import org.springframework.cloud.netflix.zuul.filters.route.okhttp.OkHttpRibbonCommandFactory; import org.springframework.cloud.netflix.zuul.web.ZuulHandlerMapping; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationListener; import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; /** * @author Spencer Gibb * @author Dave Syer */ @Configuration +@Import({ RibbonCommandFactoryConfiguration.RestClientRibbonConfiguration.class, + RibbonCommandFactoryConfiguration.OkHttpRibbonConfiguration.class, + RibbonCommandFactoryConfiguration.HttpClientRibbonConfiguration.class }) public class ZuulProxyConfiguration extends ZuulConfiguration { @SuppressWarnings("rawtypes") @@ -89,135 +79,27 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @Override @ConditionalOnMissingBean(RouteLocator.class) public DiscoveryClientRouteLocator routeLocator() { - return new DiscoveryClientRouteLocator(this.server.getServletPrefix(), - this.discovery, this.zuulProperties, this.serviceRouteMapper); - } - - @Configuration - @ConditionalOnRibbonHttpClient - protected static class HttpClientRibbonConfiguration { - - @Autowired(required = false) - private Set zuulFallbackProviders = Collections.emptySet(); - - @Bean - @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory( - SpringClientFactory clientFactory, ZuulProperties zuulProperties) { - return new HttpClientRibbonCommandFactory(clientFactory, zuulProperties, zuulFallbackProviders); - } - } - - @Configuration - @ConditionalOnRibbonRestClient - protected static class RestClientRibbonConfiguration { - - @Autowired(required = false) - private Set zuulFallbackProviders = Collections.emptySet(); - - @Bean - @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory( - SpringClientFactory clientFactory, ZuulProperties zuulProperties) { - return new RestClientRibbonCommandFactory(clientFactory, zuulProperties, - zuulFallbackProviders); - } - } - - @Configuration - @ConditionalOnRibbonOkHttpClient - @ConditionalOnClass(name = "okhttp3.OkHttpClient") - protected static class OkHttpRibbonConfiguration { - - @Autowired(required = false) - private Set zuulFallbackProviders = Collections.emptySet(); - - @Bean - @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory( - SpringClientFactory clientFactory, ZuulProperties zuulProperties) { - return new OkHttpRibbonCommandFactory(clientFactory, zuulProperties, - zuulFallbackProviders); - } - } - - @Target({ ElementType.TYPE, ElementType.METHOD }) - @Retention(RetentionPolicy.RUNTIME) - @Documented - @Conditional(OnRibbonHttpClientCondition.class) - @interface ConditionalOnRibbonHttpClient { } - - private static class OnRibbonHttpClientCondition extends AnyNestedCondition { - public OnRibbonHttpClientCondition() { - super(ConfigurationPhase.REGISTER_BEAN); - } - - @Deprecated //remove in Edgware" - @ConditionalOnProperty(name = "zuul.ribbon.httpclient.enabled", matchIfMissing = true) - static class ZuulProperty {} - - @ConditionalOnProperty(name = "ribbon.httpclient.enabled", matchIfMissing = true) - static class RibbonProperty {} - } - - @Target({ ElementType.TYPE, ElementType.METHOD }) - @Retention(RetentionPolicy.RUNTIME) - @Documented - @Conditional(OnRibbonOkHttpClientCondition.class) - @interface ConditionalOnRibbonOkHttpClient { } - - private static class OnRibbonOkHttpClientCondition extends AnyNestedCondition { - public OnRibbonOkHttpClientCondition() { - super(ConfigurationPhase.REGISTER_BEAN); - } - - @Deprecated //remove in Edgware" - @ConditionalOnProperty("zuul.ribbon.okhttp.enabled") - static class ZuulProperty {} - - @ConditionalOnProperty("ribbon.okhttp.enabled") - static class RibbonProperty {} - } - - @Target({ ElementType.TYPE, ElementType.METHOD }) - @Retention(RetentionPolicy.RUNTIME) - @Documented - @Conditional(OnRibbonRestClientCondition.class) - @interface ConditionalOnRibbonRestClient { } - - private static class OnRibbonRestClientCondition extends AnyNestedCondition { - public OnRibbonRestClientCondition() { - super(ConfigurationPhase.REGISTER_BEAN); - } - - @Deprecated //remove in Edgware" - @ConditionalOnProperty("zuul.ribbon.restclient.enabled") - static class ZuulProperty {} - - @ConditionalOnProperty("ribbon.restclient.enabled") - static class RibbonProperty {} + return new DiscoveryClientRouteLocator(this.server.getServletPrefix(), this.discovery, this.zuulProperties, + this.serviceRouteMapper); } // pre filters @Bean - public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator, - ProxyRequestHelper proxyRequestHelper) { - return new PreDecorationFilter(routeLocator, this.server.getServletPrefix(), - this.zuulProperties, proxyRequestHelper); + public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator, ProxyRequestHelper proxyRequestHelper) { + return new PreDecorationFilter(routeLocator, this.server.getServletPrefix(), this.zuulProperties, + proxyRequestHelper); } // route filters @Bean public RibbonRoutingFilter ribbonRoutingFilter(ProxyRequestHelper helper, RibbonCommandFactory ribbonCommandFactory) { - RibbonRoutingFilter filter = new RibbonRoutingFilter(helper, ribbonCommandFactory, - this.requestCustomizers); + RibbonRoutingFilter filter = new RibbonRoutingFilter(helper, ribbonCommandFactory, this.requestCustomizers); return filter; } @Bean - public SimpleHostRoutingFilter simpleHostRoutingFilter(ProxyRequestHelper helper, - ZuulProperties zuulProperties) { + public SimpleHostRoutingFilter simpleHostRoutingFilter(ProxyRequestHelper helper, ZuulProperties zuulProperties) { return new SimpleHostRoutingFilter(helper, zuulProperties); } @@ -270,8 +152,7 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { } } - private static class ZuulDiscoveryRefreshListener - implements ApplicationListener { + private static class ZuulDiscoveryRefreshListener implements ApplicationListener { private HeartbeatMonitor monitor = new HeartbeatMonitor(); From a38b7b71ac8be9608ac2530dac41cd6298d696cf Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 25 Oct 2016 12:56:49 +0100 Subject: [PATCH 4/5] Append to X-Forwarded-* headers instead of replacing them This fixes most of the issues people encounter when there are multiple proxies in the request. The tricky thing is that there is another header "Forwarded" that we don't recognize, but which backends probably do, at least some of the time (since it is from an actual RFC). The problem is that "Forwarded" does not contain the ports, so Spring UriComponentsBuilder cannot use it to rewrite links to a specific port. Since we do not support it already, this change doesn't make things any worse, but the corner case is there still. --- .../zuul/filters/pre/PreDecorationFilter.java | 30 +++++++++++++++++-- .../filters/pre/PreDecorationFilterTests.java | 17 +++++++++++ 2 files changed, 44 insertions(+), 3 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 1b5bd474..1f65218f 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 @@ -163,15 +163,39 @@ public class PreDecorationFilter extends ZuulFilter { } private void addProxyHeaders(RequestContext ctx, Route route) { - String host = toHostHeader(ctx.getRequest()); - String port = String.valueOf(ctx.getRequest().getServerPort()); - String proto = ctx.getRequest().getScheme(); + HttpServletRequest request = ctx.getRequest(); + String host = toHostHeader(request); + String port = String.valueOf(request.getServerPort()); + String proto = request.getScheme(); + if (hasHeader(request, "X-Forwarded-Host")) { + host = request.getHeader("X-Forwarded-Host") + "," + host; + if (!hasHeader(request, "X-Forwarded-Port")) { + if (hasHeader(request, "X-Forwarded-Proto")) { + StringBuilder builder = new StringBuilder(); + for (String previous : StringUtils.commaDelimitedListToStringArray(request.getHeader("X-Forwarded-Proto"))) { + if (builder.length()>0) { + builder.append(","); + } + builder.append("https".equals(previous) ? "443" : "80"); + } + builder.append(",").append(port); + port = builder.toString(); + } + } else { + port = request.getHeader("X-Forwarded-Port") + "," + port; + } + proto = request.getHeader("X-Forwarded-Proto") + "," + proto; + } ctx.addZuulRequestHeader("X-Forwarded-Host", host); ctx.addZuulRequestHeader("X-Forwarded-Port", port); ctx.addZuulRequestHeader(ZuulHeaders.X_FORWARDED_PROTO, proto); addProxyPrefix(ctx, route); } + private boolean hasHeader(HttpServletRequest request, String name) { + return StringUtils.hasLength(request.getHeader(name)); + } + private void addProxyPrefix(RequestContext ctx, Route route) { String forwardedPrefix = ctx.getRequest().getHeader("X-Forwarded-Prefix"); String contextPath = ctx.getRequest().getContextPath(); 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 940634ff..b07b43a2 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 @@ -103,6 +103,23 @@ public class PreDecorationFilterTests { assertEquals("localhost:8080", ctx.getZuulRequestHeaders().get("x-forwarded-host")); } + @Test + public void xForwardedHostAppends() throws Exception { + this.properties.setPrefix("/api"); + this.request.setRequestURI("/api/foo/1"); + this.request.setRemoteAddr("5.6.7.8"); + this.request.setServerPort(8080); + this.request.addHeader("X-Forwarded-Host", "example.com"); + this.request.addHeader("X-Forwarded-Proto", "https"); + this.routeLocator.addRoute( + new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null)); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertEquals("example.com,localhost:8080", ctx.getZuulRequestHeaders().get("x-forwarded-host")); + assertEquals("443,8080", ctx.getZuulRequestHeaders().get("x-forwarded-port")); + assertEquals("https,http", ctx.getZuulRequestHeaders().get("x-forwarded-proto")); + } + @Test public void hostHeaderSet() throws Exception { this.properties.setPrefix("/api"); From 7f075d11ee56d57fa1a38a7d02555f9b8c3ed95e Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 25 Oct 2016 14:06:16 +0100 Subject: [PATCH 5/5] Allow Cors requests through to backend by default Instead of blocking them (which is the default behaviour of a Spring MVC controller) Cors requests will flow through the Zuul filters by default. Users can control it by grabbing the ZuulHandlerMapping in a @PostConstruct and injecting some CorsConfiguration via its setCorsConfigurations() method. --- .../netflix/zuul/web/ZuulController.java | 5 ++- .../netflix/zuul/web/ZuulHandlerMapping.java | 18 ++++++-- .../ServletPathZuulProxyApplicationTests.java | 45 ++++++++++++++++--- 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulController.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulController.java index 8f5d4b1d..d8fe936d 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulController.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulController.java @@ -37,9 +37,10 @@ public class ZuulController extends ServletWrappingController { } @Override - protected ModelAndView handleRequestInternal(HttpServletRequest request, - HttpServletResponse response) throws Exception { + public ModelAndView handleRequest(HttpServletRequest request, HttpServletResponse response) throws Exception { try { + // We don't care about the other features of the base class, just want to + // handle the request return super.handleRequestInternal(request, response); } finally { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java index af81c9ae..c1e6dd54 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java @@ -25,6 +25,8 @@ 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.web.cors.CorsConfiguration; +import org.springframework.web.servlet.HandlerExecutionChain; import org.springframework.web.servlet.handler.AbstractUrlHandlerMapping; import com.netflix.zuul.context.RequestContext; @@ -51,6 +53,16 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping { setOrder(-200); } + @Override + protected HandlerExecutionChain getCorsHandlerExecutionChain(HttpServletRequest request, + HandlerExecutionChain chain, CorsConfiguration config) { + if (config == null) { + // Allow CORS requests to go to the backend + return chain; + } + return super.getCorsHandlerExecutionChain(request, chain, config); + } + public void setErrorController(ErrorController errorController) { this.errorController = errorController; } @@ -63,10 +75,8 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping { } @Override - protected Object lookupHandler(String urlPath, HttpServletRequest request) - throws Exception { - if (this.errorController != null - && urlPath.equals(this.errorController.getErrorPath())) { + protected Object lookupHandler(String urlPath, HttpServletRequest request) throws Exception { + if (this.errorController != null && urlPath.equals(this.errorController.getErrorPath())) { return null; } String[] ignored = this.routeLocator.getIgnoredPaths().toArray(new String[0]); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ServletPathZuulProxyApplicationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ServletPathZuulProxyApplicationTests.java index 9b103cce..eb14d911 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ServletPathZuulProxyApplicationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ServletPathZuulProxyApplicationTests.java @@ -18,6 +18,9 @@ package org.springframework.cloud.netflix.zuul; import static org.junit.Assert.assertEquals; +import java.net.URI; +import java.net.URISyntaxException; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -33,13 +36,16 @@ import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpEntity; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; +import org.springframework.http.RequestEntity; import org.springframework.http.ResponseEntity; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.web.bind.annotation.CrossOrigin; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.client.RestClientException; import com.netflix.zuul.context.RequestContext; @@ -68,9 +74,38 @@ public class ServletPathZuulProxyApplicationTests { public void getOnSelfViaSimpleHostRoutingFilter() { this.routes.addRoute("/self/**", "http://localhost:" + this.port + "/app/local"); this.endpoint.reset(); + ResponseEntity result = new TestRestTemplate().exchange("http://localhost:" + this.port + "/app/self/1", + HttpMethod.GET, new HttpEntity<>((Void) null), String.class); + assertEquals(HttpStatus.OK, result.getStatusCode()); + assertEquals("Gotten 1!", result.getBody()); + } + + @Test + public void optionsOnRawEndpoint() throws Exception { + ResponseEntity result = new TestRestTemplate().exchange(RequestEntity + .options(new URI("http://localhost:" + this.port + "/app/local/1")) + .header("Origin", "http://localhost:9000").header("Access-Control-Request-Method", "GET").build(), + String.class); + assertEquals(HttpStatus.OK, result.getStatusCode()); + assertEquals("http://localhost:9000", result.getHeaders().getFirst("Access-Control-Allow-Origin")); + } + + @Test + public void optionsOnSelf() throws Exception { + this.routes.addRoute("/self/**", "http://localhost:" + this.port + "/app/local"); + this.endpoint.reset(); + ResponseEntity result = new TestRestTemplate().exchange(RequestEntity + .options(new URI("http://localhost:" + this.port + "/app/self/1")) + .header("Origin", "http://localhost:9000").header("Access-Control-Request-Method", "GET").build(), + String.class); + assertEquals(HttpStatus.OK, result.getStatusCode()); + assertEquals("http://localhost:9000", result.getHeaders().getFirst("Access-Control-Allow-Origin")); + } + + @Test + public void contentOnRawEndpoint() throws Exception { ResponseEntity result = new TestRestTemplate().exchange( - "http://localhost:" + this.port + "/app/self/1", HttpMethod.GET, - new HttpEntity<>((Void) null), String.class); + RequestEntity.get(new URI("http://localhost:" + this.port + "/app/local/1")).build(), String.class); assertEquals(HttpStatus.OK, result.getStatusCode()); assertEquals("Gotten 1!", result.getBody()); } @@ -80,9 +115,8 @@ public class ServletPathZuulProxyApplicationTests { this.routes.addRoute(new ZuulRoute("strip", "/strip/**", "strip", "http://localhost:" + this.port + "/app/local", false, false, null)); this.endpoint.reset(); - ResponseEntity result = new TestRestTemplate().exchange( - "http://localhost:" + this.port + "/app/strip", HttpMethod.GET, - new HttpEntity<>((Void) null), String.class); + ResponseEntity result = new TestRestTemplate().exchange("http://localhost:" + this.port + "/app/strip", + HttpMethod.GET, new HttpEntity<>((Void) null), String.class); assertEquals(HttpStatus.OK, result.getStatusCode()); // Prefix not stripped to it goes to /local/strip assertEquals("Gotten strip!", result.getBody()); @@ -96,6 +130,7 @@ public class ServletPathZuulProxyApplicationTests { static class ServletPathZuulProxyApplication { @RequestMapping(value = "/local/{id}", method = RequestMethod.GET) + @CrossOrigin(origins = "*") public String get(@PathVariable String id) { return "Gotten " + id + "!"; }