HADOOP-13434. Add bash quoting to Shell class. (Owen O'Malley)

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java

@@ -118,6 +118,21 @@ public abstract class Shell {
     }
   }
 
+  /**
+   * Quote the given arg so that bash will interpret it as a single value.
+   * Note that this quotes it for one level of bash, if you are passing it
+   * into a badly written shell script, you need to fix your shell script.
+   * @param arg the argument to quote
+   * @return the quoted string
+   */
+  static String bashQuote(String arg) {
+    StringBuilder buffer = new StringBuilder(arg.length() + 2);
+    buffer.append('\'');
+    buffer.append(arg.replace("'", "'\\''"));
+    buffer.append('\'');
+    return buffer.toString();
+  }
+
   /** a Unix command to get the current user's name: {@value}. */
   public static final String USER_NAME_COMMAND = "whoami";
 
@@ -173,7 +188,7 @@ public abstract class Shell {
   /** a Unix command to get the current user's groups list. */
   public static String[] getGroupsCommand() {
     return (WINDOWS)? new String[]{"cmd", "/c", "groups"}
-                    : new String[]{"bash", "-c", "groups"};
+                    : new String[]{"groups"};
   }
 
   /**
@@ -184,10 +199,14 @@ public abstract class Shell {
    */
   public static String[] getGroupsForUserCommand(final String user) {
     //'groups username' command return is inconsistent across different unixes
-    return WINDOWS ?
+    if (WINDOWS) {
-      new String[]
+      return new String[]
-          {getWinUtilsPath(), "groups", "-F", "\"" + user + "\""}
+          {getWinUtilsPath(), "groups", "-F", "\"" + user + "\""};
-      : new String[] {"bash", "-c", "id -gn " + user + "; id -Gn " + user};
+    } else {
+      String quotedUser = bashQuote(user);
+      return new String[] {"bash", "-c", "id -gn " + quotedUser +
+                            "; id -Gn " + quotedUser};
+    }
   }
 
   /**
@@ -199,17 +218,20 @@ public abstract class Shell {
    */
   public static String[] getGroupsIDForUserCommand(final String user) {
     //'groups username' command return is inconsistent across different unixes
-    return WINDOWS ?
+    if (WINDOWS) {
-        new String[]
+      return new String[]{getWinUtilsPath(), "groups", "-F", "\"" + user +
-            {getWinUtilsPath(), "groups", "-F", "\"" + user + "\""}
+                           "\""};
-        : new String[] {"bash", "-c", "id -g " + user + "; id -G " + user};
+    } else {
+      String quotedUser = bashQuote(user);
+      return new String[] {"bash", "-c", "id -g " + quotedUser + "; id -G " +
+                            quotedUser};
+    }
   }
 
   /** A command to get a given netgroup's user list. */
   public static String[] getUsersForNetgroupCommand(final String netgroup) {
     //'groups username' command return is non-consistent across different unixes
-    return WINDOWS ? new String [] {"cmd", "/c", "getent netgroup " + netgroup}
+    return new String[] {"getent", "netgroup", netgroup};
-                    : new String [] {"bash", "-c", "getent netgroup " + netgroup};
   }
 
   /** Return a command to get permission information. */
@@ -240,7 +262,8 @@ public abstract class Shell {
    * @return String[] containing command and arguments
    */
   public static String[] getSetPermissionCommand(String perm,
-      boolean recursive, String file) {
+                                                 boolean recursive,
+                                                 String file) {
     String[] baseCmd = getSetPermissionCommand(perm, recursive);
     String[] cmdWithFile = Arrays.copyOf(baseCmd, baseCmd.length + 1);
     cmdWithFile[cmdWithFile.length - 1] = file;
@@ -290,9 +313,9 @@ public abstract class Shell {
 
     if (isSetsidAvailable) {
       // Use the shell-builtin as it support "--" in all Hadoop supported OSes
-      return new String[] { "bash", "-c", "kill -" + code + " -- -" + pid };
+      return new String[] {"kill", "-" + code, "--", "-" + pid};
     } else {
-      return new String[] { "bash", "-c", "kill -" + code + " " + pid };
+      return new String[] {"kill", "-" + code, pid };
     }
   }
 
@@ -343,7 +366,7 @@ public abstract class Shell {
     String absolutePath = script.getAbsolutePath();
     return WINDOWS ?
       new String[] {"cmd", "/c", absolutePath }
-      : new String[] { "/bin/bash", absolutePath };
+      : new String[] {"/bin/bash", bashQuote(absolutePath) };
   }
 
   /** a Unix command to set permission: {@value}. */

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java

@@ -240,9 +240,9 @@ public class TestShell extends Assert {
       expectedCommand =
           new String[]{getWinUtilsPath(), "task", "isAlive", anyPid };
     } else if (Shell.isSetsidAvailable) {
-      expectedCommand = new String[] { "bash", "-c", "kill -0 -- -" + anyPid };
+      expectedCommand = new String[] {"kill", "-0", "--", "-" + anyPid };
     } else {
-      expectedCommand = new String[]{ "bash", "-c", "kill -0 " + anyPid };
+      expectedCommand = new String[] {"kill", "-0", anyPid };
     }
     Assert.assertArrayEquals(expectedCommand, checkProcessAliveCommand);
   }
@@ -260,9 +260,9 @@ public class TestShell extends Assert {
       expectedCommand =
           new String[]{getWinUtilsPath(), "task", "kill", anyPid };
     } else if (Shell.isSetsidAvailable) {
-      expectedCommand = new String[] { "bash", "-c", "kill -9 -- -" + anyPid };
+      expectedCommand = new String[] {"kill", "-9", "--", "-" + anyPid };
     } else {
-      expectedCommand = new String[]{ "bash", "-c", "kill -9 " + anyPid };
+      expectedCommand = new String[] {"kill", "-9", anyPid };
     }
     Assert.assertArrayEquals(expectedCommand, checkProcessAliveCommand);
   }
@@ -466,4 +466,10 @@ public class TestShell extends Assert {
     }
   }
 
+  @Test
+  public void testBashQuote() {
+    assertEquals("'foobar'", Shell.bashQuote("foobar"));
+    assertEquals("'foo'\\''bar'", Shell.bashQuote("foo'bar"));
+    assertEquals("''\\''foo'\\''bar'\\'''", Shell.bashQuote("'foo'bar'"));
+  }
 }