From 43f7810080912a543d02609ad8d82afb44672c66 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 3 Jan 2017 12:06:49 -0500 Subject: [PATCH] Provide better error handling for invalid hostnames. Fixes #159. --- .../loadbalancer/LoadBalancerInterceptor.java | 2 ++ .../RetryLoadBalancerInterceptor.java | 2 ++ .../RetryLoadBalancerInterceptorTest.java | 20 +++++++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptor.java index 0d989ae0..8a2ee708 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerInterceptor.java @@ -23,6 +23,7 @@ import org.springframework.http.HttpRequest; import org.springframework.http.client.ClientHttpRequestExecution; import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.http.client.ClientHttpResponse; +import org.springframework.util.Assert; /** * @author Spencer Gibb @@ -42,6 +43,7 @@ public class LoadBalancerInterceptor implements ClientHttpRequestInterceptor { final ClientHttpRequestExecution execution) throws IOException { final URI originalUri = request.getURI(); String serviceName = originalUri.getHost(); + Assert.state(serviceName != null, "Request URI does not contain a valid hostname: " + originalUri); return this.loadBalancer.execute(serviceName, new LoadBalancerRequest() { @Override diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java index a5cae45f..ddc91033 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java @@ -12,6 +12,7 @@ import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.support.RetryTemplate; +import org.springframework.util.Assert; /** * @author Ryan Baxter @@ -38,6 +39,7 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto final ClientHttpRequestExecution execution) throws IOException { final URI originalUri = request.getURI(); final String serviceName = originalUri.getHost(); + Assert.state(serviceName != null, "Request URI does not contain a valid hostname: " + originalUri); LoadBalancedRetryPolicy retryPolicy = lbRetryPolicyFactory.create(serviceName, loadBalancer); retryTemplate.setRetryPolicy( diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java index 1b4ba551..794ec679 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java @@ -16,6 +16,7 @@ import org.springframework.mock.http.client.MockClientHttpResponse; import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.support.RetryTemplate; +import static org.bouncycastle.crypto.tls.ConnectionEnd.client; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.mockito.Matchers.any; @@ -69,6 +70,25 @@ public class RetryLoadBalancerInterceptorTest { verify(retryTemplate, times(1)).setRetryPolicy(any(NeverRetryPolicy.class)); } + @Test(expected = IllegalStateException.class) + public void interceptInvalidHost() throws Throwable { + HttpRequest request = mock(HttpRequest.class); + when(request.getURI()).thenReturn(new URI("http://foo_underscore")); + ClientHttpResponse clientHttpResponse = new MockClientHttpResponse(new byte[]{}, HttpStatus.OK); + LoadBalancedRetryPolicy policy = mock(LoadBalancedRetryPolicy.class); + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(request, policy, client,"foo"); + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory = mock(LoadBalancedRetryPolicyFactory.class); + when(lbRetryPolicyFactory.create(eq("foo_underscore"), any(ServiceInstanceChooser.class))).thenReturn(policy); + ServiceInstance serviceInstance = mock(ServiceInstance.class); + when(client.choose(eq("foo_underscore"))).thenReturn(serviceInstance); + when(client.execute(eq("foo_underscore"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); + lbProperties.setEnabled(true); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, retryTemplate, lbProperties, lbRetryPolicyFactory); + byte[] body = new byte[]{}; + ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); + ClientHttpResponse rsp = interceptor.intercept(request, body, execution); + } + @Test public void interceptNeverRetry() throws Throwable { HttpRequest request = mock(HttpRequest.class);