From 4f2f4f6a6072748ffe0e751e216893de238f986d Mon Sep 17 00:00:00 2001 From: Tim Ysewyn Date: Tue, 19 Dec 2017 20:08:00 +0100 Subject: [PATCH] Added util method to upgrade the connection to a secure one. (#2551) * Added util method to upgrade the connection to a secure one. - Adds support for websocket URIs - Fixes bug where a non-secure scheme other than http would be changed to https - Deprecated `updateToHttpsIfNeeded` in favor of `updateToSecureConnectionIfNeeded` * Expect http when no scheme was set * Skip update to secure connection when the uri is empty --- .../feign/ribbon/FeignLoadBalancer.java | 10 +++- .../ribbon/RibbonClientConfiguration.java | 5 +- .../ribbon/RibbonLoadBalancerClient.java | 3 +- .../cloud/netflix/ribbon/RibbonUtils.java | 60 ++++++++++++++++--- .../apache/RibbonLoadBalancingHttpClient.java | 5 +- .../okhttp/OkHttpLoadBalancingClient.java | 6 +- .../netflix/ribbon/RibbonUtilsTests.java | 20 +++++-- 7 files changed, 84 insertions(+), 25 deletions(-) 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 91c3a775..da47db68 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 @@ -43,8 +43,14 @@ import com.netflix.client.config.IClientConfig; import com.netflix.loadbalancer.ILoadBalancer; import com.netflix.loadbalancer.Server; -import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToSecureConnectionIfNeeded; +/** + * @author Dave Syer + * @author Spencer Gibb + * @author Ryan Baxter + * @author Tim Ysewyn + */ public class FeignLoadBalancer extends AbstractLoadBalancerAwareClient { @@ -101,7 +107,7 @@ public class FeignLoadBalancer extends @Override public URI reconstructURIWithServer(Server server, URI original) { - URI uri = updateToHttpsIfNeeded(original, this.clientConfig, this.serverIntrospector, server); + URI uri = updateToSecureConnectionIfNeeded(original, this.clientConfig, this.serverIntrospector, server); return super.reconstructURIWithServer(server, uri); } 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 6a38e720..dc251478 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 @@ -56,10 +56,11 @@ import com.sun.jersey.client.apache4.ApacheHttpClient4; import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses; import static org.springframework.cloud.netflix.ribbon.RibbonUtils.setRibbonProperty; -import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToSecureConnectionIfNeeded; /** * @author Dave Syer + * @author Tim Ysewyn */ @SuppressWarnings("deprecation") @Configuration @@ -192,7 +193,7 @@ public class RibbonClientConfiguration { @Override public URI reconstructURIWithServer(Server server, URI original) { - URI uri = updateToHttpsIfNeeded(original, this.config, + URI uri = updateToSecureConnectionIfNeeded(original, this.config, this.serverIntrospector, server); return super.reconstructURIWithServer(server, uri); } 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 000e7557..3c441bbe 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 @@ -34,6 +34,7 @@ import com.netflix.loadbalancer.Server; * @author Spencer Gibb * @author Dave Syer * @author Ryan Baxter + * @author Tim Ysewyn */ public class RibbonLoadBalancerClient implements LoadBalancerClient { @@ -52,7 +53,7 @@ public class RibbonLoadBalancerClient implements LoadBalancerClient { Server server = new Server(instance.getHost(), instance.getPort()); IClientConfig clientConfig = clientFactory.getClientConfig(serviceId); ServerIntrospector serverIntrospector = serverIntrospector(serviceId); - URI uri = RibbonUtils.updateToHttpsIfNeeded(original, clientConfig, + URI uri = RibbonUtils.updateToSecureConnectionIfNeeded(original, clientConfig, serverIntrospector, server); return context.reconstructURIWithServer(server, uri); } 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 index 35e85e32..464af407 100644 --- 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 @@ -2,6 +2,7 @@ package org.springframework.cloud.netflix.ribbon; import java.net.URI; +import org.springframework.util.StringUtils; import org.springframework.web.util.UriComponentsBuilder; import com.netflix.client.config.CommonClientConfigKey; @@ -11,9 +12,13 @@ import com.netflix.config.DynamicPropertyFactory; import com.netflix.config.DynamicStringProperty; import com.netflix.loadbalancer.Server; +import java.util.HashMap; +import java.util.Map; + /** * @author Spencer Gibb * @author Jacques-Etienne Beaudet + * @author Tim Ysewyn */ public class RibbonUtils { @@ -21,6 +26,14 @@ public class RibbonUtils { public static final String DEFAULT_NAMESPACE = "ribbon"; + private static final Map unsecureSchemeMapping; + static + { + unsecureSchemeMapping = new HashMap<>(); + unsecureSchemeMapping.put("http", "https"); + unsecureSchemeMapping.put("ws", "wss"); + } + public static void setRibbonProperty(String serviceId, String suffix, String value) { // how to set the namespace properly? String key = getRibbonKey(serviceId, suffix); @@ -67,20 +80,49 @@ public class RibbonUtils { * @param serverIntrospector * @param server * @return + * + * @deprecated use {@link #updateToSecureConnectionIfNeeded} */ public static URI updateToHttpsIfNeeded(URI uri, IClientConfig config, ServerIntrospector serverIntrospector, Server server) { + return updateToSecureConnectionIfNeeded(uri, config, serverIntrospector, server); + } + + /** + * Replace the scheme to the secure variant if needed. If the {@link #unsecureSchemeMapping} map contains the uri + * scheme 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 updateToSecureConnectionIfNeeded(URI uri, IClientConfig config, + ServerIntrospector serverIntrospector, Server server) { String scheme = uri.getScheme(); - if (!"".equals(uri.toString()) && !"https".equals(scheme) && isSecure(config, serverIntrospector, server)) { - UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUri(uri).scheme("https"); - if (uri.getRawQuery() != null) { - // When building the URI, UriComponentsBuilder verify the allowed characters and does not - // support the '+' so we replace it for its equivalent '%20'. - // See issue https://jira.spring.io/browse/SPR-10172 - uriComponentsBuilder.replaceQuery(uri.getRawQuery().replace("+", "%20")); - } - return uriComponentsBuilder.build(true).toUri(); + + if (StringUtils.isEmpty(scheme)) { + scheme = "http"; + } + + if (!StringUtils.isEmpty(uri.toString()) + && unsecureSchemeMapping.containsKey(scheme) + && isSecure(config, serverIntrospector, server)) { + return upgradeConnection(uri, unsecureSchemeMapping.get(scheme)); } return uri; } + + private static URI upgradeConnection(URI uri, String scheme) { + UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUri(uri).scheme(scheme); + if (uri.getRawQuery() != null) { + // When building the URI, UriComponentsBuilder verify the allowed characters and does not + // support the '+' so we replace it for its equivalent '%20'. + // See issue https://jira.spring.io/browse/SPR-10172 + uriComponentsBuilder.replaceQuery(uri.getRawQuery().replace("+", "%20")); + } + return uriComponentsBuilder.build(true).toUri(); + } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java index 1650120e..52cacdc0 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java @@ -32,11 +32,12 @@ import org.springframework.web.util.UriComponentsBuilder; import java.net.URI; -import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToSecureConnectionIfNeeded; /** * @author Christian Lohmann * @author Ryan Baxter + * @author Tim Ysewyn */ // TODO: rename (ie new class that extends this in Dalston) to ApacheHttpLoadBalancingClient public class RibbonLoadBalancingHttpClient extends @@ -85,7 +86,7 @@ public class RibbonLoadBalancingHttpClient extends @Override public URI reconstructURIWithServer(Server server, URI original) { - URI uri = updateToHttpsIfNeeded(original, this.config, this.serverIntrospector, + URI uri = updateToSecureConnectionIfNeeded(original, this.config, this.serverIntrospector, server); return super.reconstructURIWithServer(server, uri); } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java index ae319d2d..f7b325db 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java @@ -25,18 +25,18 @@ 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; import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.Response; -import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToSecureConnectionIfNeeded; /** * @author Spencer Gibb * @author Ryan Baxter + * @author Tim Ysewyn */ public class OkHttpLoadBalancingClient extends AbstractLoadBalancingClient { @@ -91,7 +91,7 @@ public class OkHttpLoadBalancingClient @Override public URI reconstructURIWithServer(Server server, URI original) { - URI uri = updateToHttpsIfNeeded(original, this.config, this.serverIntrospector, + URI uri = updateToSecureConnectionIfNeeded(original, this.config, this.serverIntrospector, server); return super.reconstructURIWithServer(server, uri); } 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 index f4d63dda..0ebbc3b7 100644 --- 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 @@ -30,11 +30,12 @@ import com.netflix.loadbalancer.Server; import static org.hamcrest.Matchers.is; import static org.springframework.cloud.netflix.ribbon.RibbonUtils.isSecure; -import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToSecureConnectionIfNeeded; /** * @author Spencer Gibb * @author Jacques-Etienne Beaudet + * @author Tim Ysewyn */ public class RibbonUtilsTests { @@ -84,28 +85,35 @@ public class RibbonUtilsTests { @Test public void uriIsNotChangedWhenServerIsNotSecured() throws URISyntaxException { URI original = new URI("http://foo"); - URI updated = updateToHttpsIfNeeded(original, NON_SECURE_CONFIG, NON_SECURE_INTROSPECTOR, SERVER); + URI updated = updateToSecureConnectionIfNeeded(original, NON_SECURE_CONFIG, NON_SECURE_INTROSPECTOR, SERVER); Assert.assertThat("URI should not have been updated since server is not secured.", original, is(updated)); } @Test public void uriIsNotChangedWhenServerIsSecuredAndUriAlreadyInHttps() throws URISyntaxException { URI original = new URI("https://foo"); - URI updated = updateToHttpsIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); + URI updated = updateToSecureConnectionIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); Assert.assertThat("URI should not have been updated since uri is already in https.", original, is(updated)); } @Test public void shouldUpgradeUriToHttpsWhenServerIsSecureAndUriNotInHttps() throws URISyntaxException { URI original = new URI("http://foo"); - URI updated = updateToHttpsIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); + URI updated = updateToSecureConnectionIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); Assert.assertThat("URI should have been updated to https.", updated, is(new URI("https://foo"))); } + @Test + public void shouldUpgradeUriToWssWhenServerIsSecureAndUriNotInWss() throws URISyntaxException { + URI original = new URI("ws://foo"); + URI updated = updateToSecureConnectionIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); + Assert.assertThat("URI should have been updated to wss.", updated, is(new URI("wss://foo"))); + } + @Test public void shouldSubstitutePlusInQueryParam() throws URISyntaxException { URI original = new URI("http://foo/%20bar?hello=1+2"); - URI updated = updateToHttpsIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); + URI updated = updateToSecureConnectionIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); Assert.assertThat("URI should have had its plus sign replaced in query string.", updated, is(new URI( "https://foo/%20bar?hello=1%202"))); } @@ -113,7 +121,7 @@ public class RibbonUtilsTests { @Test public void emptyStringUri() throws URISyntaxException { URI original = new URI(""); - URI updated = updateToHttpsIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); + URI updated = updateToSecureConnectionIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); Assert.assertThat("URI should be the emptry string", updated, is(new URI( ""))); }