From 150240cce04d7aa9445f80804b40544bebfb7fdd Mon Sep 17 00:00:00 2001
From: Kihwal Lee <kihwal@apache.org>
Date: Fri, 5 Feb 2016 15:48:32 -0600
Subject: [PATCH] HADOOP-12772. NetworkTopologyWithNodeGroup.getNodeGroup() can
 loop infinitely for invalid 'loc' values. Contributed by Kuhu Shukla.

(cherry picked from commit 49e176c29f95c179c0f6b07d4d582e6a771a96bd)
---
 hadoop-common-project/hadoop-common/CHANGES.txt   |  3 +++
 .../hadoop/net/NetworkTopologyWithNodeGroup.java  |  7 ++++++-
 .../main/java/org/apache/hadoop/net/NodeBase.java |  9 ++++++++-
 .../net/TestNetworkTopologyWithNodeGroup.java     | 15 ++++++++++++++-
 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index 53b64173abc..9956fdeeb3a 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -1023,6 +1023,9 @@ Release 2.7.3 - UNRELEASED
 
     HADOOP-12761. incremental maven build is not really incremental (sjlee)
 
+    HADOOP-12772. NetworkTopologyWithNodeGroup.getNodeGroup() can loop
+    infinitely for invalid 'loc' values (Kuhu Shukla via kihwal)
+
 Release 2.7.2 - 2016-01-25
 
   INCOMPATIBLE CHANGES
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopologyWithNodeGroup.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopologyWithNodeGroup.java
index 72031aad93a..8ebe846df15 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopologyWithNodeGroup.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopologyWithNodeGroup.java
@@ -101,7 +101,12 @@ public class NetworkTopologyWithNodeGroup extends NetworkTopology {
           return null;
         } else {
           // may be a leaf node
-          return getNodeGroup(node.getNetworkLocation());
+          if(!(node.getNetworkLocation() == null ||
+              node.getNetworkLocation().isEmpty())) {
+            return getNodeGroup(node.getNetworkLocation());
+          } else {
+            return NodeBase.ROOT;
+          }
         }
       } else {
         // not in cluster map, don't handle it
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NodeBase.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NodeBase.java
index 9f40eeaa9b2..b136297711c 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NodeBase.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NodeBase.java
@@ -127,7 +127,14 @@ public class NodeBase implements Node {
    * is not {@link #PATH_SEPARATOR}
    */
   public static String normalize(String path) {
-    if (path == null || path.length() == 0) return ROOT;
+    if (path == null) {
+      throw new IllegalArgumentException(
+          "Network Location is null ");
+    }
+
+    if (path.length() == 0) {
+      return ROOT;
+    }
     
     if (path.charAt(0) != PATH_SEPARATOR) {
       throw new IllegalArgumentException(
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetworkTopologyWithNodeGroup.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetworkTopologyWithNodeGroup.java
index 15bd9fe4924..c2c528a9c9f 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetworkTopologyWithNodeGroup.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/TestNetworkTopologyWithNodeGroup.java
@@ -178,7 +178,20 @@ public class TestNetworkTopologyWithNodeGroup {
       assertTrue(frequency.get(key) > 0 || key == dataNodes[0]);
     }
   }
-  
+
+  @Test
+  public void testNodeGroup() throws Exception {
+    String res = cluster.getNodeGroup("");
+    assertTrue("NodeGroup should be NodeBase.ROOT for empty location",
+        res.equals(NodeBase.ROOT));
+    try {
+      cluster.getNodeGroup(null);
+    } catch (IllegalArgumentException e) {
+      assertTrue("Null Network Location should throw exception!",
+          e.getMessage().contains("Network Location is null"));
+    }
+  }
+
   /**
    * This test checks that adding a node with invalid topology will be failed 
    * with an exception to show topology is invalid.