From 5e698278a83b254aefd2be2dd9f4f16395127c90 Mon Sep 17 00:00:00 2001
From: Scott Oster <scott.oster@cardinalhealth.com>
Date: Wed, 10 Aug 2016 13:10:30 -0400
Subject: [PATCH] Fixed double-encoding issue in RibbonClientConfiguration

fixes gh-1241
---
 .../ribbon/RibbonClientConfiguration.java     |  2 +-
 .../RibbonClientConfigurationTests.java       | 70 +++++++++++++++++--
 2 files changed, 64 insertions(+), 8 deletions(-)

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 efc4433e..3420d4f9 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
@@ -200,7 +200,7 @@ public class RibbonClientConfiguration {
 		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()
+				original = UriComponentsBuilder.fromUri(original).scheme("https").build(true)
 						.toUri();
 			}
 			return super.reconstructURIWithServer(server, original);
diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java
index 101121b5..31544a78 100644
--- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java
+++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java
@@ -16,26 +16,46 @@
 
 package org.springframework.cloud.netflix.ribbon;
 
-import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.assertThat;
+import java.net.URI;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration.OverrideRestClient;
 
 import com.netflix.client.config.CommonClientConfigKey;
 import com.netflix.client.config.DefaultClientConfigImpl;
 import com.netflix.client.config.IClientConfig;
-import org.junit.Test;
+import com.netflix.loadbalancer.Server;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.when;
 
 /**
  * @author Spencer Gibb
  */
 public class RibbonClientConfigurationTests {
 
-	@Test
-	public void restClientInitCalledOnce() {
-		CountingConfig config = new CountingConfig();
+	private CountingConfig config;
+
+	@Mock
+	private ServerIntrospector inspector;
+
+	@Before
+	public void setup() {
+		MockitoAnnotations.initMocks(this);
+		config = new CountingConfig();
 		config.setProperty(CommonClientConfigKey.ConnectTimeout, "1");
 		config.setProperty(CommonClientConfigKey.ReadTimeout, "1");
 		config.setProperty(CommonClientConfigKey.MaxHttpConnectionsPerHost, "1");
 		config.setClientName("testClient");
+	}
+
+	@Test
+	public void restClientInitCalledOnce() {
 		new TestRestClient(config);
 		assertThat(config.count, is(equalTo(1)));
 	}
@@ -44,7 +64,43 @@ public class RibbonClientConfigurationTests {
 		int count = 0;
 	}
 
-	static class TestRestClient extends RibbonClientConfiguration.OverrideRestClient {
+	@Test
+	public void testSecureUriFromClientConfig() throws Exception {
+		Server server = new Server("foo", 7777);
+		when(inspector.isSecure(server)).thenReturn(true);
+
+		OverrideRestClient overrideRestClient = new OverrideRestClient(this.config,
+				inspector);
+		URI uri = overrideRestClient.reconstructURIWithServer(server,
+				new URI("http://foo/"));
+		assertThat(uri, is(new URI("https://foo:7777/")));
+	}
+
+	@Test
+	public void testInSecureUriFromClientConfig() throws Exception {
+		Server server = new Server("foo", 7777);
+		when(inspector.isSecure(server)).thenReturn(false);
+
+		OverrideRestClient overrideRestClient = new OverrideRestClient(this.config,
+				inspector);
+		URI uri = overrideRestClient.reconstructURIWithServer(server,
+				new URI("http://foo/"));
+		assertThat(uri, is(new URI("http://foo:7777/")));
+	}
+
+	@Test
+	public void testNotDoubleEncodedWhenSecure() throws Exception {
+		Server server = new Server("foo", 7777);
+		when(inspector.isSecure(server)).thenReturn(true);
+
+		OverrideRestClient overrideRestClient = new OverrideRestClient(this.config,
+				inspector);
+		URI uri = overrideRestClient.reconstructURIWithServer(server,
+				new URI("http://foo/%20bar"));
+		assertThat(uri, is(new URI("https://foo:7777/%20bar")));
+	}
+
+	static class TestRestClient extends OverrideRestClient {
 
 		private TestRestClient(IClientConfig ncc) {
 			super(ncc, new DefaultServerIntrospector());