diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java index 94a2141b..1c3ed45a 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java @@ -23,7 +23,6 @@ import java.util.LinkedHashMap; import java.util.Map; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; -import org.springframework.web.util.UriComponentsBuilder; import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.ClientException; @@ -38,10 +37,11 @@ import com.netflix.loadbalancer.Server; import feign.Client; import feign.Request; -import feign.RequestTemplate; import feign.Response; import feign.Util; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; + public class FeignLoadBalancer extends AbstractLoadBalancerAwareClient { @@ -98,13 +98,8 @@ public class FeignLoadBalancer extends @Override public URI reconstructURIWithServer(Server server, URI original) { - String scheme = original.getScheme(); - if (!"https".equals(scheme) && (this.serverIntrospector.isSecure(server) - || this.clientConfig.get(CommonClientConfigKey.IsSecure, false))) { - original = UriComponentsBuilder.fromUri(original).scheme("https").build() - .toUri(); - } - return super.reconstructURIWithServer(server, original); + URI uri = updateToHttpsIfNeeded(original, this.clientConfig, this.serverIntrospector, server); + return super.reconstructURIWithServer(server, uri); } static class RibbonRequest extends ClientRequest implements Cloneable { @@ -188,4 +183,4 @@ public class FeignLoadBalancer extends } -} \ No newline at end of file +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java index 3420d4f9..3292d6a4 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java @@ -52,7 +52,8 @@ import com.sun.jersey.api.client.Client; import com.sun.jersey.client.apache4.ApacheHttpClient4; import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses; -import static org.springframework.cloud.netflix.ribbon.RibbonProperyUtils.setRibbonProperty; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.setRibbonProperty; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; /** * @author Dave Syer @@ -187,23 +188,21 @@ public class RibbonClientConfiguration { static class OverrideRestClient extends RestClient { + private IClientConfig config; private ServerIntrospector serverIntrospector; - protected OverrideRestClient(IClientConfig ncc, + protected OverrideRestClient(IClientConfig config, ServerIntrospector serverIntrospector) { super(); + this.config = config; this.serverIntrospector = serverIntrospector; - initWithNiwsConfig(ncc); + initWithNiwsConfig(this.config); } @Override public URI reconstructURIWithServer(Server server, URI original) { - String scheme = original.getScheme(); - if (!"https".equals(scheme) && this.serverIntrospector.isSecure(server)) { - original = UriComponentsBuilder.fromUri(original).scheme("https").build(true) - .toUri(); - } - return super.reconstructURIWithServer(server, original); + URI uri = updateToHttpsIfNeeded(original, this.config, this.serverIntrospector, server); + return super.reconstructURIWithServer(server, uri); } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java index 28ca7b7d..4d00a9fb 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java @@ -29,7 +29,6 @@ import org.springframework.util.Assert; import org.springframework.util.ReflectionUtils; import org.springframework.web.util.UriComponentsBuilder; -import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.IClientConfig; import com.netflix.loadbalancer.ILoadBalancer; import com.netflix.loadbalancer.Server; @@ -113,14 +112,8 @@ public class RibbonLoadBalancerClient implements LoadBalancerClient { private boolean isSecure(Server server, String serviceId) { IClientConfig config = this.clientFactory.getClientConfig(serviceId); - if (config != null) { - Boolean isSecure = config.get(CommonClientConfigKey.IsSecure); - if (isSecure != null) { - return isSecure; - } - } - - return serverIntrospector(serviceId).isSecure(server); + ServerIntrospector serverIntrospector = serverIntrospector(serviceId); + return RibbonUtils.isSecure(config, serverIntrospector, server); } protected Server getServer(String serviceId) { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonProperyUtils.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonProperyUtils.java deleted file mode 100644 index b4881ede..00000000 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonProperyUtils.java +++ /dev/null @@ -1,33 +0,0 @@ -package org.springframework.cloud.netflix.ribbon; - -import com.netflix.config.ConfigurationManager; -import com.netflix.config.DynamicPropertyFactory; -import com.netflix.config.DynamicStringProperty; - -/** - * @author Spencer Gibb - */ -public class RibbonProperyUtils { - - public static final String VALUE_NOT_SET = "__not__set__"; - - public static final String DEFAULT_NAMESPACE = "ribbon"; - - public static void setRibbonProperty(String serviceId, String suffix, String value) { - // how to set the namespace properly? - String key = getRibbonKey(serviceId, suffix); - DynamicStringProperty property = getProperty(key); - if (property.get().equals(VALUE_NOT_SET)) { - ConfigurationManager.getConfigInstance().setProperty(key, value); - } - } - - public static String getRibbonKey(String serviceId, String suffix) { - return serviceId + "." + DEFAULT_NAMESPACE + "." + suffix; - } - - public static DynamicStringProperty getProperty(String key) { - return DynamicPropertyFactory.getInstance().getStringProperty(key, VALUE_NOT_SET); - } - -} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonUtils.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonUtils.java new file mode 100644 index 00000000..711b665b --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonUtils.java @@ -0,0 +1,79 @@ +package org.springframework.cloud.netflix.ribbon; + +import com.netflix.client.config.CommonClientConfigKey; +import com.netflix.client.config.IClientConfig; +import com.netflix.client.config.IClientConfigKey; +import com.netflix.config.ConfigurationManager; +import com.netflix.config.DynamicPropertyFactory; +import com.netflix.config.DynamicStringProperty; +import com.netflix.loadbalancer.Server; +import org.springframework.web.util.UriComponentsBuilder; + +import java.net.URI; + +/** + * @author Spencer Gibb + */ +public class RibbonUtils { + + public static final String VALUE_NOT_SET = "__not__set__"; + + public static final String DEFAULT_NAMESPACE = "ribbon"; + + public static void setRibbonProperty(String serviceId, String suffix, String value) { + // how to set the namespace properly? + String key = getRibbonKey(serviceId, suffix); + DynamicStringProperty property = getProperty(key); + if (property.get().equals(VALUE_NOT_SET)) { + ConfigurationManager.getConfigInstance().setProperty(key, value); + } + } + + public static String getRibbonKey(String serviceId, String suffix) { + return serviceId + "." + DEFAULT_NAMESPACE + "." + suffix; + } + + public static DynamicStringProperty getProperty(String key) { + return DynamicPropertyFactory.getInstance().getStringProperty(key, VALUE_NOT_SET); + } + + /** + * Determine if client is secure. If the supplied {@link IClientConfig} has the {@link CommonClientConfigKey#IsSecure} + * set, return that value. Otherwise, query the supplied {@link ServerIntrospector}. + * @param config the supplied client configuration. + * @param serverIntrospector + * @param server + * @return true if the client is secure + */ + public static boolean isSecure(IClientConfig config, ServerIntrospector serverIntrospector, Server server) { + if (config != null) { + Boolean isSecure = config.get(CommonClientConfigKey.IsSecure); + if (isSecure != null) { + return isSecure; + } + } + + return serverIntrospector.isSecure(server); + } + + /** + * Replace the scheme to https if needed. If the uri doesn't start with https and + * {@link #isSecure(IClientConfig, ServerIntrospector, Server)} is true, update the scheme. + * This assumes the uri is already encoded to avoid double encoding. + * + * @param uri + * @param config + * @param serverIntrospector + * @param server + * @return + */ + public static URI updateToHttpsIfNeeded(URI uri, IClientConfig config, ServerIntrospector serverIntrospector, Server server) { + String scheme = uri.getScheme(); + if (!"https".equals(scheme) && isSecure(config, serverIntrospector, server)) { + return UriComponentsBuilder.fromUri(uri).scheme("https").build(true) + .toUri(); + } + return uri; + } + +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancerTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancerTests.java index c5a60993..e8d52c12 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancerTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancerTests.java @@ -19,6 +19,7 @@ import java.net.URI; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.Map; import org.junit.Before; import org.junit.Test; @@ -102,6 +103,28 @@ public class FeignLoadBalancerTests { assertThat(uri, is(new URI("https://foo:7777/"))); } + @Test + @SneakyThrows + public void testInsecureUriFromInsecureClientConfigToSecureServerIntrospector() { + when(this.config.get(IsSecure)).thenReturn(false); + this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, + new ServerIntrospector() { + @Override + public boolean isSecure(Server server) { + return true; + } + + @Override + public Map getMetadata(Server server) { + return null; + } + }); + Server server = new Server("foo", 7777); + URI uri = this.feignLoadBalancer.reconstructURIWithServer(server, + new URI("http://foo/")); + assertThat(uri, is(new URI("http://foo:7777/"))); + } + @Test @SneakyThrows public void testSecureUriFromClientConfigOverride() { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonUtilsTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonUtilsTests.java new file mode 100644 index 00000000..f8e8204f --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonUtilsTests.java @@ -0,0 +1,104 @@ +/* + * Copyright 2013-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.ribbon; + +import com.netflix.client.config.CommonClientConfigKey; +import org.junit.Assert; +import org.junit.Test; + +import com.netflix.client.config.DefaultClientConfigImpl; +import com.netflix.loadbalancer.Server; + +import java.util.Map; + +import static org.hamcrest.Matchers.is; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.isSecure; + +/** + * @author Spencer Gibb + */ +public class RibbonUtilsTests { + + private static final ServerIntrospector NON_SECURE_INTROSPECTOR = new StaticServerIntrospector(false); + private static final ServerIntrospector SECURE_INTROSPECTOR = new StaticServerIntrospector(true); + private static final Server SERVER = new Server("localhost", 8080); + private static final DefaultClientConfigImpl SECURE_CONFIG = getConfig(true); + private static final DefaultClientConfigImpl NON_SECURE_CONFIG = getConfig(false); + private static final DefaultClientConfigImpl NO_IS_SECURE_CONFIG = new DefaultClientConfigImpl(); + + @Test + public void noRibbonPropSecureIntrospector() { + boolean secure = isSecure(NO_IS_SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); + Assert.assertThat("isSecure was wrong", secure, is(true)); + } + + @Test + public void noRibbonPropNonSecureIntrospector() { + boolean secure = isSecure(NO_IS_SECURE_CONFIG, NON_SECURE_INTROSPECTOR, SERVER); + Assert.assertThat("isSecure was wrong", secure, is(false)); + } + + @Test + public void isSecureRibbonPropSecureIntrospector() { + boolean secure = isSecure(SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); + Assert.assertThat("isSecure was wrong", secure, is(true)); + } + + @Test + public void nonSecureRibbonPropNonSecureIntrospector() { + boolean secure = isSecure(NON_SECURE_CONFIG, NON_SECURE_INTROSPECTOR, SERVER); + Assert.assertThat("isSecure was wrong", secure, is(false)); + } + + @Test + public void isSecureRibbonPropNonSecureIntrospector() { + boolean secure = isSecure(SECURE_CONFIG, NON_SECURE_INTROSPECTOR, SERVER); + Assert.assertThat("isSecure was wrong", secure, is(true)); + } + + @Test + public void nonSecureRibbonPropSecureIntrospector() { + boolean secure = isSecure(NON_SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); + Assert.assertThat("isSecure was wrong", secure, is(false)); + } + + static DefaultClientConfigImpl getConfig(boolean value) { + DefaultClientConfigImpl config = new DefaultClientConfigImpl(); + config.setProperty(CommonClientConfigKey.IsSecure, value); + return config; + } + + static class StaticServerIntrospector implements ServerIntrospector { + + final boolean secure; + + public StaticServerIntrospector(boolean secure) { + this.secure = secure; + } + + @Override + public boolean isSecure(Server server) { + return this.secure; + } + + @Override + public Map getMetadata(Server server) { + return null; + } + } +} diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfiguration.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfiguration.java index 25fb5d25..06a1cf81 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfiguration.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfiguration.java @@ -41,7 +41,7 @@ import com.netflix.niws.loadbalancer.NIWSDiscoveryPing; import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses; import static com.netflix.client.config.CommonClientConfigKey.EnableZoneAffinity; -import static org.springframework.cloud.netflix.ribbon.RibbonProperyUtils.setRibbonProperty; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.setRibbonProperty; import lombok.extern.apachecommons.CommonsLog; diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfigurationTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfigurationTests.java index 3e6866a5..0aee7709 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfigurationTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfigurationTests.java @@ -36,10 +36,10 @@ import com.netflix.niws.loadbalancer.DiscoveryEnabledServer; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.springframework.cloud.netflix.ribbon.RibbonProperyUtils.VALUE_NOT_SET; -import static org.springframework.cloud.netflix.ribbon.RibbonProperyUtils.getProperty; -import static org.springframework.cloud.netflix.ribbon.RibbonProperyUtils.getRibbonKey; -import static org.springframework.cloud.netflix.ribbon.RibbonProperyUtils.setRibbonProperty; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.VALUE_NOT_SET; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.getProperty; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.getRibbonKey; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.setRibbonProperty; /** * @author Dave Syer