From c8829f2aa4ffac27772c4e960ced51a582f38d00 Mon Sep 17 00:00:00 2001
From: Max Ishchenko <max.ishchenko@gmail.com>
Date: Tue, 11 Oct 2016 17:19:33 +0300
Subject: [PATCH] Prevent double url encoding for secure ribbon urls (#1389)

Secure ribbon urls were forced to use https scheme via UriComponentsBuilder, that was created from original uri. This transformation url encoded previously encoded url parts that were used to create builder. This was introduced in c883495.

This change fixes double url encoding using RibbonUtils.updateToHttpsIfNeeded that fixes double escaping case and corner case with '+' in url as well.

Fixes gh-1382
---
 .../ribbon/RibbonLoadBalancerClient.java      |  9 +++----
 .../ribbon/RibbonLoadBalancerClientTests.java | 26 +++++++++++++++++++
 2 files changed, 30 insertions(+), 5 deletions(-)

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 4d00a9fb..c9261db5 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
@@ -52,11 +52,10 @@ public class RibbonLoadBalancerClient implements LoadBalancerClient {
 		RibbonLoadBalancerContext context = this.clientFactory
 				.getLoadBalancerContext(serviceId);
 		Server server = new Server(instance.getHost(), instance.getPort());
-		boolean secure = isSecure(server, serviceId);
-		URI uri = original;
-		if (secure) {
-			uri = UriComponentsBuilder.fromUri(uri).scheme("https").build().toUri();
-		}
+		IClientConfig clientConfig = clientFactory.getClientConfig(serviceId);
+		ServerIntrospector serverIntrospector = serverIntrospector(serviceId);
+		URI uri = RibbonUtils.updateToHttpsIfNeeded(original, clientConfig,
+				serverIntrospector, server);
 		return context.reconstructURIWithServer(server, uri);
 	}
 
diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java
index e224a134..f2e540cd 100644
--- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java
+++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java
@@ -38,6 +38,7 @@ import com.netflix.loadbalancer.Server;
 import com.netflix.loadbalancer.ServerStats;
 
 import lombok.SneakyThrows;
+import org.springframework.web.util.DefaultUriTemplateHandler;
 
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
@@ -107,6 +108,31 @@ public class RibbonLoadBalancerClientTests {
 		assertEquals(server.getPort(), uri.getPort());
 	}
 
+	@Test
+	public void testReconstructSecureUriWithSpecialCharsPath() {
+		testReconstructUriWithPath("https", "/foo=|");
+	}
+
+	@Test
+	public void testReconstructUnsecureUriWithSpecialCharsPath() {
+		testReconstructUriWithPath("http", "/foo=|");
+	}
+
+	private void testReconstructUriWithPath(String scheme, String path) {
+		RibbonServer server = getRibbonServer();
+		IClientConfig config = mock(IClientConfig.class);
+		when(config.get(CommonClientConfigKey.IsSecure)).thenReturn(true);
+		when(clientFactory.getClientConfig(server.getServiceId())).thenReturn(config);
+
+		RibbonLoadBalancerClient client = getRibbonLoadBalancerClient(server);
+		ServiceInstance serviceInstance = client.choose(server.getServiceId());
+
+		URI expanded = new DefaultUriTemplateHandler()
+				.expand(scheme + "://" + server.getServiceId() + path);
+		URI reconstructed = client.reconstructURI(serviceInstance, expanded);
+		assertEquals(expanded.getPath(), reconstructed.getPath());
+	}
+
 	@Test
 	@SneakyThrows
 	public void testReconstructUriWithSecureClientConfig() {