diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index d9561cdf23e..f25a14fe0f8 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/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 diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java index e06dbd7971b..fa2f59b8c7c 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/DNS.java @@ -96,42 +96,51 @@ public class DNS { * * @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 - * default interface + * If the given interface is invalid * */ public static String[] getIPs(String strInterface) throws UnknownHostException { - try { - NetworkInterface netIF = NetworkInterface.getByName(strInterface); - if (netIF == null) { - return new String[] { cachedHostAddress }; - } else { - Vector ips = new Vector(); - 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 }; + if ("default".equals(strInterface)) { + 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 ips = new Vector(); + Enumeration 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 class DNS { * 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 hosts = new Vector(); - 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 @@ public class DNS { try { localhost = InetAddress.getLocalHost().getCanonicalHostName(); } catch (UnknownHostException e) { - LOG.info("Unable to determine local hostname " - + "-falling back to \"" + LOCALHOST + "\"", e); + LOG.warn("Unable to determine local hostname " + + "-falling back to \"" + LOCALHOST + "\"", e); localhost = LOCALHOST; } return localhost; @@ -204,7 +214,7 @@ public class DNS { 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(); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestDNS.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestDNS.java index 5825ecf8c63..dcfc015201e 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestDNS.java +++ b/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 class TestDNS extends TestCase { * hence that caching is being used * @throws Exception if hostname lookups fail */ + @Test public void testGetLocalHostIsFast() throws Exception { - String hostname = DNS.getDefaultHost(DEFAULT); - assertNotNull(hostname); - long t1 = System.currentTimeMillis(); + String hostname1 = DNS.getDefaultHost(DEFAULT); + assertNotNull(hostname1); 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); - long interval2 = t3 - t2; + assertEquals(hostname2, hostname1); + long interval = t2 - t1; assertTrue( - "It is taking to long to determine the local host -caching is not working", - interval2 < 20000); + "Took too long to determine local host - caching is not working", + 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 @@ public class TestDNS extends TestCase { } /** - * Test that passing a null pointer is as the interface - * fails with a NullPointerException - * @throws Exception if something went wrong + * Test null interface name */ + @Test public void testNullInterface() throws Exception { try { String host = DNS.getDefaultHost(null); fail("Expected a NullPointerException, got " + host); - } catch (NullPointerException expected) { - //this is expected + } catch (NullPointerException npe) { + // 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 - * back - * @throws Exception if something went wrong + * Get the IP addresses of an unknown interface */ + @Test public void testIPsOfUnknownInterface() throws Exception { - String[] ips = DNS.getIPs("name-of-an-unknown-interface"); - assertNotNull(ips); - assertTrue(ips.length > 0); + try { + DNS.getIPs("name-of-an-unknown-interface"); + 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 class TestDNS extends TestCase { * 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);