Validate hostname (serviceId) in @FeignClient

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

@@ -17,6 +17,8 @@
 package org.springframework.cloud.netflix.feign;
 
 import java.lang.annotation.Annotation;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
@@ -117,10 +119,21 @@ public class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar,
 	}
 
 	private String getServiceId(Map<String, Object> attributes) {
-		if (StringUtils.hasText((String) attributes.get("name"))) {
-			return (String) attributes.get("name");
+		String name = (String) attributes.get("name");
+		if (!StringUtils.hasText(name)) {
+			name = (String) attributes.get("value");
 		}
-		return (String) attributes.get("value");
+		if (!StringUtils.hasText(name)) {
+			return "";
+		}
+		String host = null;
+		try {
+			host = new URI("http://" + name).getHost();
+		}
+		catch (URISyntaxException e) {
+		}
+		Assert.state(host != null, "Service id not legal hostname (" + name + ")");
+		return name;
 	}
 
 	private String getUrl(Map<String, Object> attributes) {

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

@@ -38,7 +38,7 @@ public class RibbonClientConfigurationRegistrar implements ImportBeanDefinitionR
 		if (attrs != null && attrs.containsKey("value")) {
 			AnnotationAttributes[] clients = (AnnotationAttributes[]) attrs.get("value");
 			for (AnnotationAttributes client : clients) {
-				registerClientConfiguration(registry, client.get("name"),
+				registerClientConfiguration(registry, getClientName(client),
 						client.get("configuration"));
 			}
 		}
@@ -60,11 +60,10 @@ public class RibbonClientConfigurationRegistrar implements ImportBeanDefinitionR
 			return null;
 		}
 		String value = (String) client.get("value");
-		if (value != null && StringUtils.hasText(value)) {
-			return value;
+		if (!StringUtils.hasText(value)) {
+			value = (String) client.get("name");
 		}
-		value = (String) client.get("name");
-		if (value != null && StringUtils.hasText(value)) {
+		if (StringUtils.hasText(value)) {
 			return value;
 		}
 		throw new IllegalStateException(

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/invalid/FeignClientValidationTests.java

+/*
+ * 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.invalid;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.springframework.cloud.netflix.feign.EnableFeignClients;
+import org.springframework.cloud.netflix.feign.FeignClient;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+
+import static org.junit.Assert.assertNotNull;
+
+/**
+ * @author Dave Syer
+ */
+public class FeignClientValidationTests {
+
+	@Rule
+	public ExpectedException expected = ExpectedException.none();
+
+	@Test
+	public void invalid() {
+		this.expected.expectMessage("not legal hostname (foo_bar)");
+		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
+				BadConfiguration.class);
+		assertNotNull(context.getBean(BadConfiguration.Client.class));
+		context.close();
+	}
+
+	@Configuration
+	@EnableFeignClients()
+	protected static class BadConfiguration {
+
+		@FeignClient("foo_bar")
+		interface Client {
+			@RequestMapping(method = RequestMethod.GET, value = "/")
+			String get();
+		}
+
+	}
+
+}

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientValidationTests.java

+/*
+ * 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.valid;
+
+import org.junit.Test;
+import org.springframework.cloud.netflix.feign.EnableFeignClients;
+import org.springframework.cloud.netflix.feign.FeignClient;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
+
+import static org.junit.Assert.assertNotNull;
+
+/**
+ * @author Dave Syer
+ */
+public class FeignClientValidationTests {
+
+	@Test
+	public void valid() {
+		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
+				GoodConfiguration.class);
+		assertNotNull(context.getBean(GoodConfiguration.Client.class));
+		context.close();
+	}
+
+	@Configuration
+	@EnableFeignClients
+	protected static class GoodConfiguration {
+
+		@FeignClient("foo")
+		interface Client {
+			@RequestMapping(method = RequestMethod.GET, value = "/")
+			String get();
+		}
+
+	}
+
+}