diff --git a/hadoop-common/CHANGES.txt b/hadoop-common/CHANGES.txt index b1a332fa1c3..01fcd4acade 100644 --- a/hadoop-common/CHANGES.txt +++ b/hadoop-common/CHANGES.txt @@ -313,6 +313,9 @@ Trunk (unreleased changes) HADOOP-7472. RPC client should deal with IP address change. (Kihwal Lee via suresh) + + HADOOP-7499. Add method for doing a sanity check on hostnames in NetUtils. + (Jeffrey Naisbit via mahadev) OPTIMIZATIONS diff --git a/hadoop-common/src/main/java/org/apache/hadoop/net/CachedDNSToSwitchMapping.java b/hadoop-common/src/main/java/org/apache/hadoop/net/CachedDNSToSwitchMapping.java index a9d6b7f2dff..5bd3ca35de8 100644 --- a/hadoop-common/src/main/java/org/apache/hadoop/net/CachedDNSToSwitchMapping.java +++ b/hadoop-common/src/main/java/org/apache/hadoop/net/CachedDNSToSwitchMapping.java @@ -17,9 +17,6 @@ */ package org.apache.hadoop.net; -import java.net.InetAddress; -import java.net.SocketException; -import java.net.UnknownHostException; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -91,32 +88,6 @@ public class CachedDNSToSwitchMapping implements DNSToSwitchMapping { return result; } - /** - * Resolves host names and adds them to the cache. - * Unlike the 'resolve" method, this won't hide UnknownHostExceptions - * - * @param names to resolve - * @return List of resolved names - * @throws UnknownHostException if any hosts cannot be resolved - */ - public List resolveValidHosts(List names) - throws UnknownHostException { - if (names.isEmpty()) { - return new ArrayList(); - } - List addresses = new ArrayList(names.size()); - for (String name : names) { - addresses.add(InetAddress.getByName(name).getHostAddress()); - } - - List uncachedHosts = this.getUncachedHosts(names); - - // Resolve the uncached hosts - List resolvedHosts = rawMapping.resolveValidHosts(uncachedHosts); - this.cacheResolvedHosts(uncachedHosts, resolvedHosts); - return this.getCachedHosts(addresses); - } - public List resolve(List names) { // normalize all input names to be in the form of IP addresses names = NetUtils.normalizeHostNames(names); diff --git a/hadoop-common/src/main/java/org/apache/hadoop/net/DNSToSwitchMapping.java b/hadoop-common/src/main/java/org/apache/hadoop/net/DNSToSwitchMapping.java index 3f0f7b3834a..f9816becb6c 100644 --- a/hadoop-common/src/main/java/org/apache/hadoop/net/DNSToSwitchMapping.java +++ b/hadoop-common/src/main/java/org/apache/hadoop/net/DNSToSwitchMapping.java @@ -18,7 +18,6 @@ package org.apache.hadoop.net; import java.util.List; -import java.net.UnknownHostException; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -45,23 +44,4 @@ public interface DNSToSwitchMapping { * @return list of resolved network paths */ public List resolve(List names); - - /** - * Resolves a list of DNS-names/IP-addresses and returns back a list of - * switch information (network paths). One-to-one correspondence must be - * maintained between the elements in the lists. - * Consider an element in the argument list - x.y.com. The switch information - * that is returned must be a network path of the form /foo/rack, - * where / is the root, and 'foo' is the switch where 'rack' is connected. - * Note the hostname/ip-address is not part of the returned path. - * The network topology of the cluster would determine the number of - * components in the network path. Unlike 'resolve', names must be - * resolvable - * @param names - * @return list of resolved network paths - * @throws UnknownHostException if any hosts are not resolvable - */ - public List resolveValidHosts(List names) - throws UnknownHostException; - } diff --git a/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java b/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java index 4a9a7fe1a1b..b22aaa009c1 100644 --- a/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java +++ b/hadoop-common/src/main/java/org/apache/hadoop/net/NetUtils.java @@ -27,6 +27,7 @@ import java.net.Socket; import java.net.SocketAddress; import java.net.SocketException; import java.net.URI; +import java.net.URISyntaxException; import java.net.UnknownHostException; import java.net.ConnectException; import java.nio.channels.SocketChannel; @@ -428,6 +429,35 @@ public class NetUtils { return hostNames; } + /** + * Performs a sanity check on the list of hostnames/IPs to verify they at least + * appear to be valid. + * @param names - List of hostnames/IPs + * @throws UnknownHostException + */ + public static void verifyHostnames(String[] names) throws UnknownHostException { + for (String name: names) { + if (name == null) { + throw new UnknownHostException("null hostname found"); + } + // The first check supports URL formats (e.g. hdfs://, etc.). + // java.net.URI requires a schema, so we add a dummy one if it doesn't + // have one already. + URI uri = null; + try { + uri = new URI(name); + if (uri.getHost() == null) { + uri = new URI("http://" + name); + } + } catch (URISyntaxException e) { + uri = null; + } + if (uri == null || uri.getHost() == null) { + throw new UnknownHostException(name + " is not a valid Inet address"); + } + } + } + private static final Pattern ipPattern = // Pattern for matching hostname to ip:port Pattern.compile("\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}:?\\d*"); diff --git a/hadoop-common/src/main/java/org/apache/hadoop/net/ScriptBasedMapping.java b/hadoop-common/src/main/java/org/apache/hadoop/net/ScriptBasedMapping.java index 85dbe201aa0..e3fe4581bb1 100644 --- a/hadoop-common/src/main/java/org/apache/hadoop/net/ScriptBasedMapping.java +++ b/hadoop-common/src/main/java/org/apache/hadoop/net/ScriptBasedMapping.java @@ -20,7 +20,6 @@ package org.apache.hadoop.net; import java.util.*; import java.io.*; -import java.net.UnknownHostException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -123,17 +122,6 @@ implements Configurable return m; } - - public List resolveValidHosts(List names) - throws UnknownHostException { - List result = this.resolve(names); - if (result != null) { - return result; - } else { - throw new UnknownHostException( - "Unknown host(s) returned from ScriptBasedMapping"); - } - } private String runResolveCommand(List args) { int loopCount = 0; diff --git a/hadoop-common/src/test/java/org/apache/hadoop/net/StaticMapping.java b/hadoop-common/src/test/java/org/apache/hadoop/net/StaticMapping.java index 0974aa335d6..c3923ed9510 100644 --- a/hadoop-common/src/test/java/org/apache/hadoop/net/StaticMapping.java +++ b/hadoop-common/src/test/java/org/apache/hadoop/net/StaticMapping.java @@ -18,7 +18,6 @@ package org.apache.hadoop.net; import java.util.*; -import java.net.UnknownHostException; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.conf.Configured; @@ -60,19 +59,4 @@ public class StaticMapping extends Configured implements DNSToSwitchMapping { return m; } } - public List resolveValidHosts(List names) - throws UnknownHostException { - List m = new ArrayList(); - synchronized (nameToRackMap) { - for (String name : names) { - String rackId; - if ((rackId = nameToRackMap.get(name)) != null) { - m.add(rackId); - } else { - throw new UnknownHostException(name); - } - } - return m; - } - } } diff --git a/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetUtils.java b/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetUtils.java index 2aad71cb52e..f49d4c886ec 100644 --- a/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetUtils.java +++ b/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetUtils.java @@ -72,4 +72,20 @@ public class TestNetUtils { assertNull(NetUtils.getLocalInetAddress("invalid-address-for-test")); assertNull(NetUtils.getLocalInetAddress(null)); } + + @Test(expected=UnknownHostException.class) + public void testVerifyHostnamesException() throws UnknownHostException { + String[] names = {"valid.host.com", "1.com", "invalid host here"}; + NetUtils.verifyHostnames(names); + } + + @Test + public void testVerifyHostnamesNoException() { + String[] names = {"valid.host.com", "1.com"}; + try { + NetUtils.verifyHostnames(names); + } catch (UnknownHostException e) { + fail("NetUtils.verifyHostnames threw unexpected UnknownHostException"); + } + } } diff --git a/hadoop-common/src/test/java/org/apache/hadoop/net/TestScriptBasedMapping.java b/hadoop-common/src/test/java/org/apache/hadoop/net/TestScriptBasedMapping.java index b05be5ab464..fc4a781cb5e 100644 --- a/hadoop-common/src/test/java/org/apache/hadoop/net/TestScriptBasedMapping.java +++ b/hadoop-common/src/test/java/org/apache/hadoop/net/TestScriptBasedMapping.java @@ -19,7 +19,6 @@ package org.apache.hadoop.net; import java.util.ArrayList; import java.util.List; -import java.net.UnknownHostException; import org.apache.hadoop.conf.Configuration; @@ -49,37 +48,5 @@ public class TestScriptBasedMapping extends TestCase { List result = mapping.resolve(names); assertNull(result); } - - public void testResolveValidInvalidHostException() { - names = new ArrayList(); - names.add("1.com"); // Add invalid hostname that doesn't resolve - boolean exceptionThrown = false; - try { - mapping.resolveValidHosts(names); - } catch (UnknownHostException e) { - exceptionThrown = true; - } - assertTrue( - "resolveValidHosts did not throw UnknownHostException for invalid host", - exceptionThrown); - } - public void testResolveValidHostNoException() { - conf.setInt(ScriptBasedMapping.SCRIPT_ARG_COUNT_KEY, - ScriptBasedMapping.MIN_ALLOWABLE_ARGS); - conf.set(ScriptBasedMapping.SCRIPT_FILENAME_KEY, "echo"); - mapping.setConf(conf); - - names = new ArrayList(); - names.add("some.machine.name"); - names.add("other.machine.name"); - - boolean exceptionThrown = false; - try { - mapping.resolveValidHosts(names); - } catch (UnknownHostException e) { - exceptionThrown = true; - } - assertFalse("resolveValidHosts threw Exception for valid host", exceptionThrown); - } }