You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by ar...@apache.org on 2016/08/02 20:50:28 UTC
[2/3] hadoop git commit: HADOOP-13434. Add bash quoting to Shell
class. (Owen O'Malley)
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/158dd2d5
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/158dd2d5
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/158dd2d5
Branch: refs/heads/branch-2
Commit: 158dd2d5a2c76b796a19256b7d19db419b6dec5f
Parents: dc0a17e
Author: Arpit Agarwal <ar...@apache.org>
Authored: Tue Aug 2 13:40:33 2016 -0700
Committer: Arpit Agarwal <ar...@apache.org>
Committed: Tue Aug 2 13:40:56 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/158dd2d5/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 f8da08d..3007d32 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
@@ -848,7 +871,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;
@@ -877,7 +900,7 @@ public abstract class Shell {
}
builder.redirectErrorStream(redirectErrorStream);
-
+
if (Shell.WINDOWS) {
synchronized (WindowsProcessLaunchLock) {
// To workaround the race condition issue with child processes
@@ -898,7 +921,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 =
@@ -936,7 +959,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
@@ -1073,13 +1096,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;
@@ -1106,7 +1129,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.
@@ -1120,7 +1143,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) {
@@ -1145,7 +1168,7 @@ public abstract class Shell {
+ StringUtils.join(" ", command));
}
}
- this.run();
+ this.run();
}
@Override
@@ -1198,7 +1221,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() {
@@ -1207,15 +1230,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.
@@ -1237,7 +1260,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();
@@ -1275,7 +1298,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/158dd2d5/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 b0b54d0..c0491fac 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
@@ -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'"));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org