HADOOP-8154. DNS#getIPs shouldn't silently return the local host IP for bogus interface names. Contributed by Eli Collins

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

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -270,6 +270,9 @@ Release 0.23.3 - UNRELEASED
     HADOOP-7888. TestFailoverProxy fails intermittently on trunk. (Jason Lowe
     via atm)
 
+    HADOOP-8154. DNS#getIPs shouldn't silently return the local host
+    IP for bogus interface names. (eli)
+
 Release 0.23.2 - UNRELEASED 
 
   INCOMPATIBLE CHANGES

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java

@@ -96,42 +96,51 @@ public static String reverseDns(InetAddress hostIp, String ns)
    * 
    * @param strInterface
    *            The name of the network interface to query (e.g. eth0)
+   *            or the string "default"
    * @return A string vector of all the IPs associated with the provided
-   *         interface
+   *         interface. The local host IP is returned if the interface
+   *         name "default" is specified or there is an I/O error looking
+   *         for the given interface.
    * @throws UnknownHostException
-   *             If an UnknownHostException is encountered in querying the
+   *             If the given interface is invalid
-   *             default interface
    * 
    */
   public static String[] getIPs(String strInterface)
     throws UnknownHostException {
-    try {
+    if ("default".equals(strInterface)) {
-      NetworkInterface netIF = NetworkInterface.getByName(strInterface);
+      return new String[] { cachedHostAddress };
-      if (netIF == null) {
-        return new String[] { cachedHostAddress };
-      } else {
-        Vector<String> ips = new Vector<String>();
-        Enumeration e = netIF.getInetAddresses();
-        while (e.hasMoreElements()) {
-          ips.add(((InetAddress) e.nextElement()).getHostAddress());
-        }
-        return ips.toArray(new String[] {});
-      }
-    } catch (SocketException e) {
-      return new String[]  { cachedHostAddress };
     }
+    NetworkInterface netIF;
+    try {
+      netIF = NetworkInterface.getByName(strInterface);
+    } catch (SocketException e) {
+      LOG.warn("I/O error finding interface " + strInterface +
+          ": " + e.getMessage());
+      return new String[] { cachedHostAddress };
+    }
+    if (netIF == null) {
+      throw new UnknownHostException("No such interface " + strInterface);
+    }
+    Vector<String> ips = new Vector<String>();
+    Enumeration<InetAddress> e = netIF.getInetAddresses();
+    while (e.hasMoreElements()) {
+      ips.add(e.nextElement().getHostAddress());
+    }
+    return ips.toArray(new String[] {});
   }
 
 
-    /**
+  /**
    * Returns the first available IP address associated with the provided
-   * network interface
+   * network interface or the local host IP if "default" is given.
    *
    * @param strInterface
    *            The name of the network interface to query (e.g. eth0)
-   * @return The IP address in text form
+   *            or the string "default"
+   * @return The IP address in text form, the local host IP is returned
+   *         if the interface name "default" is specified
    * @throws UnknownHostException
-   *             If one is encountered in querying the default interface
+   *             If the given interface is invalid
    */
   public static String getDefaultIP(String strInterface)
     throws UnknownHostException {
@@ -149,21 +158,22 @@ public static String getDefaultIP(String strInterface)
    *            The DNS host name
    * @return A string vector of all host names associated with the IPs tied to
    *         the specified interface
-   * @throws UnknownHostException if the hostname cannot be determined
+   * @throws UnknownHostException if the given interface is invalid
    */
   public static String[] getHosts(String strInterface, String nameserver)
     throws UnknownHostException {
     String[] ips = getIPs(strInterface);
     Vector<String> hosts = new Vector<String>();
-    for (int ctr = 0; ctr < ips.length; ctr++)
+    for (int ctr = 0; ctr < ips.length; ctr++) {
       try {
         hosts.add(reverseDns(InetAddress.getByName(ips[ctr]),
                              nameserver));
       } catch (UnknownHostException ignored) {
       } catch (NamingException ignored) {
       }
-
+    }
     if (hosts.isEmpty()) {
+      LOG.warn("Unable to determine hostname for interface " + strInterface);
       return new String[] { cachedHostname };
     } else {
       return hosts.toArray(new String[hosts.size()]);
@@ -181,8 +191,8 @@ private static String resolveLocalHostname() {
     try {
       localhost = InetAddress.getLocalHost().getCanonicalHostName();
     } catch (UnknownHostException e) {
-      LOG.info("Unable to determine local hostname "
+      LOG.warn("Unable to determine local hostname "
-              + "-falling back to \"" + LOCALHOST + "\"", e);
+          + "-falling back to \"" + LOCALHOST + "\"", e);
       localhost = LOCALHOST;
     }
     return localhost;
@@ -204,7 +214,7 @@ private static String resolveLocalHostIPAddress() {
       try {
         address = InetAddress.getLocalHost().getHostAddress();
       } catch (UnknownHostException e) {
-        LOG.info("Unable to determine address of the host"
+        LOG.warn("Unable to determine address of the host"
                 + "-falling back to \"" + LOCALHOST + "\" address", e);
         try {
           address = InetAddress.getByName(LOCALHOST).getHostAddress();

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestDNS.java

@@ -16,39 +16,32 @@
  * limitations under the License.
  */
 
-
 package org.apache.hadoop.net;
 
-import junit.framework.TestCase;
-
 import java.net.UnknownHostException;
 import java.net.InetAddress;
 
+import javax.naming.NameNotFoundException;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import javax.naming.NameNotFoundException;
+import org.junit.Test;
+import static org.junit.Assert.*;
 
 /**
- *
+ * Test host name and IP resolution and caching.
  */
-public class TestDNS extends TestCase {
+public class TestDNS {
 
   private static final Log LOG = LogFactory.getLog(TestDNS.class);
   private static final String DEFAULT = "default";
 
-  /**
-   * Constructs a test case with the given name.
-   *
-   * @param name test name
-   */
-  public TestDNS(String name) {
-    super(name);
-  }
-
   /**
    * Test that asking for the default hostname works
-   * @throws Exception if hostname lookups fail   */
+   * @throws Exception if hostname lookups fail
+   */
+  @Test
   public void testGetLocalHost() throws Exception {
     String hostname = DNS.getDefaultHost(DEFAULT);
     assertNotNull(hostname);
@@ -59,26 +52,27 @@ public void testGetLocalHost() throws Exception {
    * hence that caching is being used
    * @throws Exception if hostname lookups fail
    */
+  @Test
   public void testGetLocalHostIsFast() throws Exception {
-    String hostname = DNS.getDefaultHost(DEFAULT);
+    String hostname1 = DNS.getDefaultHost(DEFAULT);
-    assertNotNull(hostname);
+    assertNotNull(hostname1);
-    long t1 = System.currentTimeMillis();
     String hostname2 = DNS.getDefaultHost(DEFAULT);
-    long t2 = System.currentTimeMillis();
+    long t1 = System.currentTimeMillis();
     String hostname3 = DNS.getDefaultHost(DEFAULT);
-    long t3 = System.currentTimeMillis();
+    long t2 = System.currentTimeMillis();
     assertEquals(hostname3, hostname2);
-    assertEquals(hostname2, hostname);
+    assertEquals(hostname2, hostname1);
-    long interval2 = t3 - t2;
+    long interval = t2 - t1;
     assertTrue(
-            "It is taking to long to determine the local host -caching is not working",
+        "Took too long to determine local host - caching is not working",
-            interval2 < 20000);
+        interval < 20000);
   }
 
   /**
    * Test that our local IP address is not null
    * @throws Exception if something went wrong
    */
+  @Test
   public void testLocalHostHasAnAddress() throws Exception {
     assertNotNull(getLocalIPAddr());
   }
@@ -90,34 +84,54 @@ private InetAddress getLocalIPAddr() throws UnknownHostException {
   }
 
   /**
-   * Test that passing a null pointer is as the interface
+   * Test null interface name
-   * fails with a NullPointerException
-   * @throws Exception if something went wrong
    */
+  @Test
   public void testNullInterface() throws Exception {
     try {
       String host = DNS.getDefaultHost(null);
       fail("Expected a NullPointerException, got " + host);
-    } catch (NullPointerException expected) {
+    } catch (NullPointerException npe) {
-      //this is expected
+      // Expected
+    }
+    try {
+      String ip = DNS.getDefaultIP(null);
+      fail("Expected a NullPointerException, got " + ip);
+    } catch (NullPointerException npe) {
+      // Expected
     }
   }
 
   /**
-   * Get the IP addresses of an unknown interface, expect to get something
+   * Get the IP addresses of an unknown interface
-   * back
-   * @throws Exception if something went wrong
    */
+  @Test
   public void testIPsOfUnknownInterface() throws Exception {
-    String[] ips = DNS.getIPs("name-of-an-unknown-interface");
+    try {
-    assertNotNull(ips);
+      DNS.getIPs("name-of-an-unknown-interface");
-    assertTrue(ips.length > 0);
+      fail("Got an IP for a bogus interface");
+    } catch (UnknownHostException e) {
+      assertEquals("No such interface name-of-an-unknown-interface",
+          e.getMessage());
+    }
+  }
+
+  /**
+   * Test the "default" IP addresses is the local IP addr
+   */
+  @Test
+  public void testGetIPWithDefault() throws Exception {
+    String[] ips = DNS.getIPs(DEFAULT);
+    assertEquals("Should only return 1 default IP", 1, ips.length);
+    assertEquals(getLocalIPAddr().getHostAddress(), ips[0].toString());
+    String ip = DNS.getDefaultIP(DEFAULT);
+    assertEquals(ip, ips[0].toString());
   }
 
   /**
    * TestCase: get our local address and reverse look it up
-   * @throws Exception if that fails
    */
+  @Test
   public void testRDNS() throws Exception {
     InetAddress localhost = getLocalIPAddr();
     try {
@@ -140,8 +154,8 @@ public void testRDNS() throws Exception {
    * Test that the name "localhost" resolves to something.
    *
    * If this fails, your machine's network is in a mess, go edit /etc/hosts
-   * @throws Exception for any problems
    */
+  @Test
   public void testLocalhostResolves() throws Exception {
     InetAddress localhost = InetAddress.getByName("localhost");
     assertNotNull("localhost is null", localhost);