New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HADOOP-13434. Add bash quoting to Shell class. #119

Closed
wants to merge 2 commits into
base: trunk
from
Jump to file or symbol
Failed to load files and symbols.
+72 −43
Diff settings

Always

Just for now

Next

HADOOP-13434. Add bash quoting to Shell class.

  • Loading branch information...
omalley committed Jul 28, 2016
commit 76aa6639475e97b36bd9e1c04b575bd8d4a08849
@@ -42,7 +42,7 @@
* 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 static void checkWindowsCommandLineLength(String...commands)
}
}
/**
* 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 @@ private static OSType getOSType() {
/** 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 @@ private static OSType getOSType() {
*/
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 @@ private static OSType getOSType() {
*/
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 @@ private static OSType getOSType() {
/**
* 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 @@ private static OSType getOSType() {
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 static String getEnvironmentVariableRegex() {
* 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
@@ -343,7 +366,7 @@ public static String appendScriptExtension(String basename) {
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}. */
@@ -527,11 +550,11 @@ private static File getHadoopHomeDir() throws FileNotFoundException {
/**
* 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 @@ protected void run() throws IOException {
}
/** 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 @@ private void runCommand() throws IOException {
}
builder.redirectErrorStream(redirectErrorStream);
if (Shell.WINDOWS) {
synchronized (WindowsProcessLaunchLock) {
// To workaround the race condition issue with child processes
@@ -894,7 +917,7 @@ private void runCommand() throws IOException {
//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 void run() {
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 String toString() {
/**
* 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 ShellCommandExecutor(String[] execString, File dir,
/**
* 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 ShellCommandExecutor(String[] execString, File dir,
* @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 void execute() throws IOException {
+ StringUtils.join(" ", command));
}
}
this.run();
this.run();
}
@Override
@@ -1194,7 +1217,7 @@ public void close() {
/**
* 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 boolean isTimedOut() {
/**
* 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 static String execCommand(String ... cmd) throws IOException {
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 void run() {
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();
@@ -235,9 +235,9 @@ public void testGetCheckProcessIsAliveCommand() throws Exception {
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 void testGetSignalKillCommand() throws Exception {
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 @@ private void assertExContains(Exception ex, String expectedText)
}
}
@Test
public void testBashQuote() {
assertEquals("'foobar'", Shell.bashQuote("foobar"));
assertEquals("'foo'\\''bar'", Shell.bashQuote("foo'bar"));
assertEquals("''\\''foo'\\''bar'\\'''", Shell.bashQuote("'foo'bar'"));
}
}
ProTip! Use n and p to navigate between commits in a pull request.