HADOOP-13434. Add bash quoting to Shell class. (Owen O'Malley)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/954465e7 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/954465e7 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/954465e7 Branch: refs/heads/YARN-2915 Commit: 954465e7ba3af3e5b083b6251562e5e77529f906 Parents: 5e5b879 Author: Arpit Agarwal <a...@apache.org> Authored: Tue Aug 2 13:40:33 2016 -0700 Committer: Arpit Agarwal <a...@apache.org> Committed: Tue Aug 2 13:40:33 2016 -0700 ---------------------------------------------------------------------- .../main/java/org/apache/hadoop/util/Shell.java | 103 ++++++++++++------- .../java/org/apache/hadoop/util/TestShell.java | 14 ++- 2 files changed, 73 insertions(+), 44 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/954465e7/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java index ea8db07..60f8ff5 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java @@ -42,7 +42,7 @@ import org.slf4j.LoggerFactory; * A base class for running a Shell command. * * <code>Shell</code> can be used to run shell commands like <code>du</code> or - * <code>df</code>. It also offers facilities to gate commands by + * <code>df</code>. It also offers facilities to gate commands by * time-intervals. */ @InterfaceAudience.Public @@ -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 ? - new String[] - {getWinUtilsPath(), "groups", "-F", "\"" + user + "\""} - : new String[] {"bash", "-c", "id -gn " + user + "; id -Gn " + user}; + if (WINDOWS) { + return new String[] + {getWinUtilsPath(), "groups", "-F", "\"" + 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 ? - new String[] - {getWinUtilsPath(), "groups", "-F", "\"" + user + "\""} - : new String[] {"bash", "-c", "id -g " + user + "; id -G " + user}; + if (WINDOWS) { + return new String[]{getWinUtilsPath(), "groups", "-F", "\"" + 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} - : new String [] {"bash", "-c", "getent netgroup " + netgroup}; + return new String[] {"getent", "netgroup", netgroup}; } /** Return a command to get permission information. */ @@ -233,14 +255,15 @@ public abstract class Shell { /** * Return a command to set permission for specific file. - * + * * @param perm String permission to set * @param recursive boolean true to apply to all sub-directories recursively * @param file String file to set * @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 }; } } @@ -310,7 +333,7 @@ public abstract class Shell { * Returns a File referencing a script with the given basename, inside the * given parent directory. The file extension is inferred by platform: * <code>".cmd"</code> on Windows, or <code>".sh"</code> otherwise. - * + * * @param parent File parent directory * @param basename String script file basename * @return File referencing the script in the directory @@ -342,8 +365,8 @@ public abstract class Shell { public static String[] getRunScriptCommand(File script) { String absolutePath = script.getAbsolutePath(); return WINDOWS ? - new String[] { "cmd", "/c", absolutePath } - : new String[] { "/bin/bash", absolutePath }; + new String[] {"cmd", "/c", absolutePath } + : new String[] {"/bin/bash", bashQuote(absolutePath) }; } /** a Unix command to set permission: {@value}. */ @@ -527,11 +550,11 @@ public abstract class Shell { /** * Fully qualify the path to a binary that should be in a known hadoop - * bin location. This is primarily useful for disambiguating call-outs - * to executable sub-components of Hadoop to avoid clashes with other - * executables that may be in the path. Caveat: this call doesn't - * just format the path to the bin directory. It also checks for file - * existence of the composed path. The output of this call should be + * bin location. This is primarily useful for disambiguating call-outs + * to executable sub-components of Hadoop to avoid clashes with other + * executables that may be in the path. Caveat: this call doesn't + * just format the path to the bin directory. It also checks for file + * existence of the composed path. The output of this call should be * cached by callers. * * @param executable executable @@ -851,7 +874,7 @@ public abstract class Shell { } /** Run the command. */ - private void runCommand() throws IOException { + private void runCommand() throws IOException { ProcessBuilder builder = new ProcessBuilder(getExecString()); Timer timeOutTimer = null; ShellTimeoutTimerTask timeoutTimerTask = null; @@ -873,7 +896,7 @@ public abstract class Shell { } builder.redirectErrorStream(redirectErrorStream); - + if (Shell.WINDOWS) { synchronized (WindowsProcessLaunchLock) { // To workaround the race condition issue with child processes @@ -894,7 +917,7 @@ public abstract class Shell { //One time scheduling. timeOutTimer.schedule(timeoutTimerTask, timeOutInterval); } - final BufferedReader errReader = + final BufferedReader errReader = new BufferedReader(new InputStreamReader( process.getErrorStream(), Charset.defaultCharset())); BufferedReader inReader = @@ -932,7 +955,7 @@ public abstract class Shell { parseExecResult(inReader); // parse the output // clear the input stream buffer String line = inReader.readLine(); - while(line != null) { + while(line != null) { line = inReader.readLine(); } // wait for the process to finish and check the exit code @@ -1069,13 +1092,13 @@ public abstract class Shell { /** * A simple shell command executor. - * + * * <code>ShellCommandExecutor</code>should be used in cases where the output * of the command needs no explicit parsing and where the command, working * directory and the environment remains unchanged. The output of the command * is stored as-is and is expected to be small. */ - public static class ShellCommandExecutor extends Shell + public static class ShellCommandExecutor extends Shell implements CommandExecutor { private String[] command; @@ -1102,7 +1125,7 @@ public abstract class Shell { /** * Create a new instance of the ShellCommandExecutor to execute a command. - * + * * @param execString The command to execute with arguments * @param dir If not-null, specifies the directory which should be set * as the current working directory for the command. @@ -1116,7 +1139,7 @@ public abstract class Shell { * @param inheritParentEnv Indicates if the process should inherit the env * vars from the parent process or not. */ - public ShellCommandExecutor(String[] execString, File dir, + public ShellCommandExecutor(String[] execString, File dir, Map<String, String> env, long timeout, boolean inheritParentEnv) { command = execString.clone(); if (dir != null) { @@ -1141,7 +1164,7 @@ public abstract class Shell { + StringUtils.join(" ", command)); } } - this.run(); + this.run(); } @Override @@ -1194,7 +1217,7 @@ public abstract class Shell { /** * To check if the passed script to shell command executor timed out or * not. - * + * * @return if the script timed out. */ public boolean isTimedOut() { @@ -1203,15 +1226,15 @@ public abstract class Shell { /** * Declare that the command has timed out. - * + * */ private void setTimedOut() { this.timedOut.set(true); } - /** - * Static method to execute a shell command. - * Covers most of the simple cases without requiring the user to implement + /** + * Static method to execute a shell command. + * Covers most of the simple cases without requiring the user to implement * the <code>Shell</code> interface. * @param cmd shell command to execute. * @return the output of the executed command. @@ -1233,7 +1256,7 @@ public abstract class Shell { public static String execCommand(Map<String, String> env, String[] cmd, long timeout) throws IOException { - ShellCommandExecutor exec = new ShellCommandExecutor(cmd, null, env, + ShellCommandExecutor exec = new ShellCommandExecutor(cmd, null, env, timeout); exec.execute(); return exec.getOutput(); @@ -1271,7 +1294,7 @@ public abstract class Shell { p.exitValue(); } catch (Exception e) { //Process has not terminated. - //So check if it has completed + //So check if it has completed //if not just destroy it. if (p != null && !shell.completed.get()) { shell.setTimedOut(); http://git-wip-us.apache.org/repos/asf/hadoop/blob/954465e7/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java index 16ec867..acc7f1d 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java @@ -235,9 +235,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); } @@ -255,9 +255,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); } @@ -461,4 +461,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'")); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
