Retain scheme (e.g. https) in URL from @FeignClient

When the URL is passed down to Ribbon and reconstructed from
a LoadBalancer the Server only knows about host and port, so the scheme
has to come from the original declaration.

There's still a potential problem with Eureka remote services that
are available with a non-secure port as well (probably fairly rare).

Fixes gh-221

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClient.java

@@ -24,6 +24,8 @@ import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLSocketFactory;
 
 import lombok.Data;
+
+import org.springframework.cloud.netflix.feign.ribbon.RibbonLoadBalancer.RibbonRequest;
 import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
 import org.springframework.util.ReflectionUtils;
 
@@ -36,8 +38,6 @@ import feign.Client;
 import feign.Request;
 import feign.Response;
 
-import static org.springframework.cloud.netflix.feign.ribbon.RibbonLoadBalancer.RibbonRequest;
-
 /**
  * @author Julien Roy
  * @author Spencer Gibb
@@ -59,12 +59,13 @@ public class FeignRibbonClient implements Client {
 			URI asUri = URI.create(request.url());
 			String clientName = asUri.getHost();
 			URI uriWithoutSchemeAndPort = URI.create(request.url().replace(
-					asUri.getScheme() + "://" + asUri.getHost(), ""));
-			RibbonLoadBalancer.RibbonRequest ribbonRequest = new RibbonRequest(
-					request, uriWithoutSchemeAndPort);
+					asUri.getScheme() + "://" + asUri.getHost(),
+					asUri.getScheme() + "://"));
+			RibbonLoadBalancer.RibbonRequest ribbonRequest = new RibbonRequest(request,
+					uriWithoutSchemeAndPort);
 			LBClient client = getClient(clientName);
-			return client.getLoadBalancer().executeWithLoadBalancer(ribbonRequest, client.config)
-					.toResponse();
+			return client.getLoadBalancer()
+					.executeWithLoadBalancer(ribbonRequest, client.config).toResponse();
 		}
 		catch (ClientException ex) {
 			if (ex.getCause() instanceof IOException) {
@@ -84,7 +85,8 @@ public class FeignRibbonClient implements Client {
 	private LBClient getClient(String clientName) {
 		IClientConfig config = this.factory.getClientConfig(clientName);
 		ILoadBalancer lb = this.factory.getLoadBalancer(clientName);
-		RibbonLoadBalancer loadBalancer = new RibbonLoadBalancer(this.defaultClient, lb, config);
+		RibbonLoadBalancer loadBalancer = new RibbonLoadBalancer(this.defaultClient, lb,
+				config);
 		return new LBClient(config, loadBalancer);
 	}
 

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientRetryTests.java

@@ -16,8 +16,6 @@
 
 package org.springframework.cloud.netflix.feign.ribbon;
 
-import static org.junit.Assert.*;
-
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Proxy;
 import java.util.Arrays;
@@ -38,7 +36,6 @@ import org.springframework.boot.test.SpringApplicationConfiguration;
 import org.springframework.cloud.netflix.feign.EnableFeignClients;
 import org.springframework.cloud.netflix.feign.FeignClient;
 import org.springframework.cloud.netflix.ribbon.RibbonClient;
-import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.test.annotation.DirtiesContext;
@@ -52,6 +49,10 @@ import com.netflix.loadbalancer.BaseLoadBalancer;
 import com.netflix.loadbalancer.ILoadBalancer;
 import com.netflix.loadbalancer.Server;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
 /**
  * @author Spencer Gibb
  */
@@ -59,10 +60,8 @@ import com.netflix.loadbalancer.Server;
 @SpringApplicationConfiguration(classes = FeignRibbonClientRetryTests.Application.class)
 @WebAppConfiguration
 @IntegrationTest({ "server.port=0", "spring.application.name=feignclienttest",
-		"localapp.ribbon.MaxAutoRetries=5",
-		"localapp.ribbon.MaxAutoRetriesNextServer=5",
-		"localapp.ribbon.OkToRetryOnAllOperations=true",
-})
+		"localapp.ribbon.MaxAutoRetries=5", "localapp.ribbon.MaxAutoRetriesNextServer=5",
+		"localapp.ribbon.OkToRetryOnAllOperations=true", })
 @DirtiesContext
 public class FeignRibbonClientRetryTests {
 
@@ -98,17 +97,17 @@ public class FeignRibbonClientRetryTests {
 
 		@RequestMapping(method = RequestMethod.GET, value = "/retryme")
 		public int retryMe() {
-			return retries.getAndIncrement();
+			return this.retries.getAndIncrement();
 		}
 
 		public static void main(String[] args) throws InterruptedException {
-			ConfigurableApplicationContext context = new SpringApplicationBuilder(Application.class).properties(
+			new SpringApplicationBuilder(Application.class).properties(
 					"spring.application.name=feignclienttest",
 					"localapp.ribbon.MaxAutoRetries=5",
 					"localapp.ribbon.MaxAutoRetriesNextServer=5",
 					"localapp.ribbon.OkToRetryOnAllOperations=true",
 					"management.contextPath=/admin"
-					//,"local.server.port=9999"
+			// ,"local.server.port=9999"
 					).run(args);
 		}
 	}
@@ -126,8 +125,8 @@ public class FeignRibbonClientRetryTests {
 	public void testRetries() {
 		int retryMe = this.testClient.retryMe();
 		assertEquals("retryCount didn't match", retryMe, 1);
-		//TODO: not sure how to verify retry happens.  Debugging through it, it works
-		//maybe the assertEquals above is enough because of the bogus servers
+		// TODO: not sure how to verify retry happens. Debugging through it, it works
+		// maybe the assertEquals above is enough because of the bogus servers
 	}
 
 	@Data
@@ -151,8 +150,7 @@ class LocalRibbonClientConfiguration {
 		BaseLoadBalancer balancer = new BaseLoadBalancer();
 		balancer.setServersList(Arrays.asList(new Server("___mybadhost__", 10001),
 				new Server("___mybadhost2__", 10002),
-				new Server("___mybadhost3__", 10003),
-				new Server("localhost", this.port)));
+				new Server("___mybadhost3__", 10003), new Server("localhost", this.port)));
 		return balancer;
 	}
 

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright 2013-2015 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.feign.ribbon;
+
+import org.hamcrest.CustomMatcher;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Matchers;
+import org.mockito.Mockito;
+import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
+import org.springframework.test.util.ReflectionTestUtils;
+
+import com.netflix.client.config.CommonClientConfigKey;
+import com.netflix.client.config.DefaultClientConfigImpl;
+import com.netflix.client.config.IClientConfig;
+import com.netflix.loadbalancer.ILoadBalancer;
+import com.netflix.loadbalancer.Server;
+
+import feign.Client;
+import feign.Request;
+import feign.Request.Options;
+import feign.RequestTemplate;
+
+/**
+ * @author Dave Syer
+ */
+public class FeignRibbonClientTests {
+
+	private ILoadBalancer loadBalancer = Mockito.mock(ILoadBalancer.class);
+	private Client delegate = Mockito.mock(Client.class);
+
+	private SpringClientFactory factory = new SpringClientFactory() {
+		@Override
+		public IClientConfig getClientConfig(String name) {
+			DefaultClientConfigImpl config = new DefaultClientConfigImpl();
+			config.set(CommonClientConfigKey.ConnectTimeout, 1000);
+			config.set(CommonClientConfigKey.ReadTimeout, 500);
+			return config;
+		}
+
+		@Override
+		public ILoadBalancer getLoadBalancer(String name) {
+			return FeignRibbonClientTests.this.loadBalancer;
+		}
+	};
+
+	private FeignRibbonClient client = new FeignRibbonClient(this.factory);
+
+	@Before
+	public void init() {
+		ReflectionTestUtils.setField(this.client, "defaultClient", this.delegate);
+		Mockito.when(this.loadBalancer.chooseServer(Matchers.any())).thenReturn(
+				new Server("foo.com", 8000));
+	}
+
+	@Test
+	public void remoteRequestIsSent() throws Exception {
+		Request request = new RequestTemplate().method("GET").append("http://foo/")
+				.request();
+		this.client.execute(request, new Options());
+		RequestMatcher matcher = new RequestMatcher("http://foo.com:8000/");
+		Mockito.verify(this.delegate).execute(Matchers.argThat(matcher),
+				Matchers.any(Options.class));
+	}
+
+	@Test
+	public void remoteRequestIsSecure() throws Exception {
+		Request request = new RequestTemplate().method("GET").append("https://foo/")
+				.request();
+		this.client.execute(request, new Options());
+		RequestMatcher matcher = new RequestMatcher("https://foo.com:8000/");
+		Mockito.verify(this.delegate).execute(Matchers.argThat(matcher),
+				Matchers.any(Options.class));
+	}
+
+	private final static class RequestMatcher extends CustomMatcher<Request> {
+		private String url;
+
+		private RequestMatcher(String url) {
+			super("request has URI: " + url);
+			this.url = url;
+		}
+
+		@Override
+		public boolean matches(Object item) {
+			Request request = (Request) item;
+			return request.url().equals(this.url);
+		}
+	}
+
+}