Commit 4f2f4f6a by Tim Ysewyn Committed by Ryan Baxter

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
parent 1210db9a
...@@ -43,8 +43,14 @@ import com.netflix.client.config.IClientConfig; ...@@ -43,8 +43,14 @@ import com.netflix.client.config.IClientConfig;
import com.netflix.loadbalancer.ILoadBalancer; import com.netflix.loadbalancer.ILoadBalancer;
import com.netflix.loadbalancer.Server; 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 public class FeignLoadBalancer extends
AbstractLoadBalancerAwareClient<FeignLoadBalancer.RibbonRequest, FeignLoadBalancer.RibbonResponse> { AbstractLoadBalancerAwareClient<FeignLoadBalancer.RibbonRequest, FeignLoadBalancer.RibbonResponse> {
...@@ -101,7 +107,7 @@ public class FeignLoadBalancer extends ...@@ -101,7 +107,7 @@ public class FeignLoadBalancer extends
@Override @Override
public URI reconstructURIWithServer(Server server, URI original) { 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); return super.reconstructURIWithServer(server, uri);
} }
......
...@@ -56,10 +56,11 @@ import com.sun.jersey.client.apache4.ApacheHttpClient4; ...@@ -56,10 +56,11 @@ import com.sun.jersey.client.apache4.ApacheHttpClient4;
import static com.netflix.client.config.CommonClientConfigKey.DeploymentContextBasedVipAddresses; 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.setRibbonProperty;
import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttpsIfNeeded; import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToSecureConnectionIfNeeded;
/** /**
* @author Dave Syer * @author Dave Syer
* @author Tim Ysewyn
*/ */
@SuppressWarnings("deprecation") @SuppressWarnings("deprecation")
@Configuration @Configuration
...@@ -192,7 +193,7 @@ public class RibbonClientConfiguration { ...@@ -192,7 +193,7 @@ public class RibbonClientConfiguration {
@Override @Override
public URI reconstructURIWithServer(Server server, URI original) { public URI reconstructURIWithServer(Server server, URI original) {
URI uri = updateToHttpsIfNeeded(original, this.config, URI uri = updateToSecureConnectionIfNeeded(original, this.config,
this.serverIntrospector, server); this.serverIntrospector, server);
return super.reconstructURIWithServer(server, uri); return super.reconstructURIWithServer(server, uri);
} }
......
...@@ -34,6 +34,7 @@ import com.netflix.loadbalancer.Server; ...@@ -34,6 +34,7 @@ import com.netflix.loadbalancer.Server;
* @author Spencer Gibb * @author Spencer Gibb
* @author Dave Syer * @author Dave Syer
* @author Ryan Baxter * @author Ryan Baxter
* @author Tim Ysewyn
*/ */
public class RibbonLoadBalancerClient implements LoadBalancerClient { public class RibbonLoadBalancerClient implements LoadBalancerClient {
...@@ -52,7 +53,7 @@ public class RibbonLoadBalancerClient implements LoadBalancerClient { ...@@ -52,7 +53,7 @@ public class RibbonLoadBalancerClient implements LoadBalancerClient {
Server server = new Server(instance.getHost(), instance.getPort()); Server server = new Server(instance.getHost(), instance.getPort());
IClientConfig clientConfig = clientFactory.getClientConfig(serviceId); IClientConfig clientConfig = clientFactory.getClientConfig(serviceId);
ServerIntrospector serverIntrospector = serverIntrospector(serviceId); ServerIntrospector serverIntrospector = serverIntrospector(serviceId);
URI uri = RibbonUtils.updateToHttpsIfNeeded(original, clientConfig, URI uri = RibbonUtils.updateToSecureConnectionIfNeeded(original, clientConfig,
serverIntrospector, server); serverIntrospector, server);
return context.reconstructURIWithServer(server, uri); return context.reconstructURIWithServer(server, uri);
} }
......
...@@ -2,6 +2,7 @@ package org.springframework.cloud.netflix.ribbon; ...@@ -2,6 +2,7 @@ package org.springframework.cloud.netflix.ribbon;
import java.net.URI; import java.net.URI;
import org.springframework.util.StringUtils;
import org.springframework.web.util.UriComponentsBuilder; import org.springframework.web.util.UriComponentsBuilder;
import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.CommonClientConfigKey;
...@@ -11,9 +12,13 @@ import com.netflix.config.DynamicPropertyFactory; ...@@ -11,9 +12,13 @@ import com.netflix.config.DynamicPropertyFactory;
import com.netflix.config.DynamicStringProperty; import com.netflix.config.DynamicStringProperty;
import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.Server;
import java.util.HashMap;
import java.util.Map;
/** /**
* @author Spencer Gibb * @author Spencer Gibb
* @author Jacques-Etienne Beaudet * @author Jacques-Etienne Beaudet
* @author Tim Ysewyn
*/ */
public class RibbonUtils { public class RibbonUtils {
...@@ -21,6 +26,14 @@ public class RibbonUtils { ...@@ -21,6 +26,14 @@ public class RibbonUtils {
public static final String DEFAULT_NAMESPACE = "ribbon"; public static final String DEFAULT_NAMESPACE = "ribbon";
private static final Map<String, String> unsecureSchemeMapping;
static
{
unsecureSchemeMapping = new HashMap<>();
unsecureSchemeMapping.put("http", "https");
unsecureSchemeMapping.put("ws", "wss");
}
public static void setRibbonProperty(String serviceId, String suffix, String value) { public static void setRibbonProperty(String serviceId, String suffix, String value) {
// how to set the namespace properly? // how to set the namespace properly?
String key = getRibbonKey(serviceId, suffix); String key = getRibbonKey(serviceId, suffix);
...@@ -67,20 +80,49 @@ public class RibbonUtils { ...@@ -67,20 +80,49 @@ public class RibbonUtils {
* @param serverIntrospector * @param serverIntrospector
* @param server * @param server
* @return * @return
*
* @deprecated use {@link #updateToSecureConnectionIfNeeded}
*/ */
public static URI updateToHttpsIfNeeded(URI uri, IClientConfig config, ServerIntrospector serverIntrospector, public static URI updateToHttpsIfNeeded(URI uri, IClientConfig config, ServerIntrospector serverIntrospector,
Server server) { 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(); String scheme = uri.getScheme();
if (!"".equals(uri.toString()) && !"https".equals(scheme) && isSecure(config, serverIntrospector, server)) {
UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUri(uri).scheme("https"); if (StringUtils.isEmpty(scheme)) {
if (uri.getRawQuery() != null) { scheme = "http";
// 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 if (!StringUtils.isEmpty(uri.toString())
uriComponentsBuilder.replaceQuery(uri.getRawQuery().replace("+", "%20")); && unsecureSchemeMapping.containsKey(scheme)
} && isSecure(config, serverIntrospector, server)) {
return uriComponentsBuilder.build(true).toUri(); return upgradeConnection(uri, unsecureSchemeMapping.get(scheme));
} }
return uri; 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();
}
} }
...@@ -32,11 +32,12 @@ import org.springframework.web.util.UriComponentsBuilder; ...@@ -32,11 +32,12 @@ import org.springframework.web.util.UriComponentsBuilder;
import java.net.URI; 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 Christian Lohmann
* @author Ryan Baxter * @author Ryan Baxter
* @author Tim Ysewyn
*/ */
// TODO: rename (ie new class that extends this in Dalston) to ApacheHttpLoadBalancingClient // TODO: rename (ie new class that extends this in Dalston) to ApacheHttpLoadBalancingClient
public class RibbonLoadBalancingHttpClient extends public class RibbonLoadBalancingHttpClient extends
...@@ -85,7 +86,7 @@ public class RibbonLoadBalancingHttpClient extends ...@@ -85,7 +86,7 @@ public class RibbonLoadBalancingHttpClient extends
@Override @Override
public URI reconstructURIWithServer(Server server, URI original) { public URI reconstructURIWithServer(Server server, URI original) {
URI uri = updateToHttpsIfNeeded(original, this.config, this.serverIntrospector, URI uri = updateToSecureConnectionIfNeeded(original, this.config, this.serverIntrospector,
server); server);
return super.reconstructURIWithServer(server, uri); return super.reconstructURIWithServer(server, uri);
} }
......
...@@ -25,18 +25,18 @@ import org.springframework.web.util.UriComponentsBuilder; ...@@ -25,18 +25,18 @@ import org.springframework.web.util.UriComponentsBuilder;
import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.CommonClientConfigKey;
import com.netflix.client.config.IClientConfig; import com.netflix.client.config.IClientConfig;
import com.netflix.loadbalancer.ILoadBalancer;
import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.Server;
import okhttp3.OkHttpClient; import okhttp3.OkHttpClient;
import okhttp3.Request; import okhttp3.Request;
import okhttp3.Response; 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 Spencer Gibb
* @author Ryan Baxter * @author Ryan Baxter
* @author Tim Ysewyn
*/ */
public class OkHttpLoadBalancingClient public class OkHttpLoadBalancingClient
extends AbstractLoadBalancingClient<OkHttpRibbonRequest, OkHttpRibbonResponse, OkHttpClient> { extends AbstractLoadBalancingClient<OkHttpRibbonRequest, OkHttpRibbonResponse, OkHttpClient> {
...@@ -91,7 +91,7 @@ public class OkHttpLoadBalancingClient ...@@ -91,7 +91,7 @@ public class OkHttpLoadBalancingClient
@Override @Override
public URI reconstructURIWithServer(Server server, URI original) { public URI reconstructURIWithServer(Server server, URI original) {
URI uri = updateToHttpsIfNeeded(original, this.config, this.serverIntrospector, URI uri = updateToSecureConnectionIfNeeded(original, this.config, this.serverIntrospector,
server); server);
return super.reconstructURIWithServer(server, uri); return super.reconstructURIWithServer(server, uri);
} }
......
...@@ -30,11 +30,12 @@ import com.netflix.loadbalancer.Server; ...@@ -30,11 +30,12 @@ import com.netflix.loadbalancer.Server;
import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.is;
import static org.springframework.cloud.netflix.ribbon.RibbonUtils.isSecure; 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 Spencer Gibb
* @author Jacques-Etienne Beaudet * @author Jacques-Etienne Beaudet
* @author Tim Ysewyn
*/ */
public class RibbonUtilsTests { public class RibbonUtilsTests {
...@@ -84,28 +85,35 @@ public class RibbonUtilsTests { ...@@ -84,28 +85,35 @@ public class RibbonUtilsTests {
@Test @Test
public void uriIsNotChangedWhenServerIsNotSecured() throws URISyntaxException { public void uriIsNotChangedWhenServerIsNotSecured() throws URISyntaxException {
URI original = new URI("http://foo"); 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)); Assert.assertThat("URI should not have been updated since server is not secured.", original, is(updated));
} }
@Test @Test
public void uriIsNotChangedWhenServerIsSecuredAndUriAlreadyInHttps() throws URISyntaxException { public void uriIsNotChangedWhenServerIsSecuredAndUriAlreadyInHttps() throws URISyntaxException {
URI original = new URI("https://foo"); 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)); Assert.assertThat("URI should not have been updated since uri is already in https.", original, is(updated));
} }
@Test @Test
public void shouldUpgradeUriToHttpsWhenServerIsSecureAndUriNotInHttps() throws URISyntaxException { public void shouldUpgradeUriToHttpsWhenServerIsSecureAndUriNotInHttps() throws URISyntaxException {
URI original = new URI("http://foo"); 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"))); Assert.assertThat("URI should have been updated to https.", updated, is(new URI("https://foo")));
} }
@Test @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 { public void shouldSubstitutePlusInQueryParam() throws URISyntaxException {
URI original = new URI("http://foo/%20bar?hello=1+2"); 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( Assert.assertThat("URI should have had its plus sign replaced in query string.", updated, is(new URI(
"https://foo/%20bar?hello=1%202"))); "https://foo/%20bar?hello=1%202")));
} }
...@@ -113,7 +121,7 @@ public class RibbonUtilsTests { ...@@ -113,7 +121,7 @@ public class RibbonUtilsTests {
@Test @Test
public void emptyStringUri() throws URISyntaxException { public void emptyStringUri() throws URISyntaxException {
URI original = new URI(""); 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( Assert.assertThat("URI should be the emptry string", updated, is(new URI(
""))); "")));
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment