SpringCloud/spring-cloud-openfeign
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Make moving to https urls consistent.

Browse Source 
This fixes a case where the ribbon property is set `ribbon.IsSecure=false`, but because the eureka secure port was enabled, the uri created by ribbon was prefixed with https when it shouldn't have.

All places where isSecure is need have been consolidated into RibbonUtils.

fixes gh-1270
This commit is contained in:
Spencer Gibb
2016-08-22 17:16:13 -06:00
parent 8d6aeebb61
commit 43adc84dfd
9 changed files with 226 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java
View File

@@ -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<FeignLoadBalancer.RibbonRequest, FeignLoadBalancer.RibbonResponse> {
@@ -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
}
}
}

17
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java
View File

@@ -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

11
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java
View File

@@ -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) {

33
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonProperyUtils.java
View File

@@ -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);
}
}

79
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonUtils.java Normal file
View File

@@ -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;
}
}

23
spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancerTests.java
View File

@@ -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<String, String> 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() {

104
spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonUtilsTests.java Normal file
View File

@@ -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<String, String> getMetadata(Server server) {
return null;
}
}
}

2
spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfiguration.java
View File

@@ -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;

8
spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfigurationTests.java
View File

@@ -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