HADOOP-7499. Add method for doing a sanity check on hostnames in NetUtils. (Jeffrey Naisbit via mahadev)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1156809 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common/CHANGES.txt

@@ -314,6 +314,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
   
     HADOOP-7333. Performance improvement in PureJavaCrc32. (Eric Caspole

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<String> resolveValidHosts(List<String> names) 
-    throws UnknownHostException {
-    if (names.isEmpty()) {
-      return new ArrayList<String>();
-    }
-    List<String> addresses = new ArrayList<String>(names.size());
-    for (String name : names) {
-      addresses.add(InetAddress.getByName(name).getHostAddress());
-    }
-
-    List<String> uncachedHosts = this.getUncachedHosts(names);
-
-    // Resolve the uncached hosts
-    List<String> resolvedHosts = rawMapping.resolveValidHosts(uncachedHosts);
-    this.cacheResolvedHosts(uncachedHosts, resolvedHosts);
-    return this.getCachedHosts(addresses);
-  }
-
   public List<String> resolve(List<String> names) {
     // normalize all input names to be in the form of IP addresses
     names = NetUtils.normalizeHostNames(names);

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<String> resolve(List<String> 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<String> resolveValidHosts(List<String> names)
-    throws UnknownHostException;
-
 }

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*");
   

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;
@@ -124,17 +123,6 @@ implements Configurable
     return m;
   }
 
-  public List<String> resolveValidHosts(List<String> names) 
-    throws UnknownHostException {
-    List<String> result = this.resolve(names);
-    if (result != null) {
-      return result;
-    } else {
-      throw new UnknownHostException(
-          "Unknown host(s) returned from ScriptBasedMapping");
-    }
-  }
-
   private String runResolveCommand(List<String> args) {
     int loopCount = 0;
     if (args.size() == 0) {

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<String> resolveValidHosts(List<String> names)
-    throws UnknownHostException {
-    List<String> m = new ArrayList<String>();
-    synchronized (nameToRackMap) {
-      for (String name : names) {
-        String rackId;
-        if ((rackId = nameToRackMap.get(name)) != null) {
-          m.add(rackId);
-        } else {
-          throw new UnknownHostException(name);
-        }
-      }
-      return m;
-    }
-  }
 }

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");
+    }
+  }
 }

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;
 
@@ -50,36 +49,4 @@ public class TestScriptBasedMapping extends TestCase {
     assertNull(result);
   }
 
-  public void testResolveValidInvalidHostException() {
-    names = new ArrayList<String>();
-    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<String>();
-    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);
-  }
 }