8012453: (process) Runtime.exec(String) fails if command contains spaces [win]
Reviewed-by: alanb
--- a/jdk/src/share/classes/java/lang/ProcessBuilder.java Tue May 14 14:32:15 2013 +0100
+++ b/jdk/src/share/classes/java/lang/ProcessBuilder.java Tue May 14 20:16:21 2013 +0400
@@ -29,7 +29,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
-import java.io.FileOutputStream;
import java.security.AccessControlException;
import java.util.Arrays;
import java.util.ArrayList;
@@ -1024,10 +1023,10 @@
dir,
redirects,
redirectErrorStream);
- } catch (IOException e) {
+ } catch (IOException | IllegalArgumentException e) {
String exceptionInfo = ": " + e.getMessage();
Throwable cause = e;
- if (security != null) {
+ if ((e instanceof IOException) && security != null) {
// Can not disclose the fail reason for read-protected files.
try {
security.checkRead(prog);
--- a/jdk/src/windows/classes/java/lang/ProcessImpl.java Tue May 14 14:32:15 2013 +0100
+++ b/jdk/src/windows/classes/java/lang/ProcessImpl.java Tue May 14 20:16:21 2013 +0400
@@ -37,7 +37,10 @@
import java.lang.ProcessBuilder.Redirect;
import java.security.AccessController;
import java.security.PrivilegedAction;
+import java.util.ArrayList;
import java.util.concurrent.TimeUnit;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
/* This class is for the exclusive use of ProcessBuilder.start() to
* create new processes.
@@ -145,6 +148,66 @@
}
+ private static class LazyPattern {
+ // Escape-support version:
+ // "(\")((?:\\\\\\1|.)+?)\\1|([^\\s\"]+)";
+ private static final Pattern PATTERN =
+ Pattern.compile("[^\\s\"]+|\"[^\"]*\"");
+ };
+
+ /* Parses the command string parameter into the executable name and
+ * program arguments.
+ *
+ * The command string is broken into tokens. The token separator is a space
+ * or quota character. The space inside quotation is not a token separator.
+ * There are no escape sequences.
+ */
+ private static String[] getTokensFromCommand(String command) {
+ ArrayList<String> matchList = new ArrayList<>(8);
+ Matcher regexMatcher = LazyPattern.PATTERN.matcher(command);
+ while (regexMatcher.find())
+ matchList.add(regexMatcher.group());
+ return matchList.toArray(new String[matchList.size()]);
+ }
+
+ private static String createCommandLine(boolean isCmdFile,
+ final String executablePath,
+ final String cmd[])
+ {
+ StringBuilder cmdbuf = new StringBuilder(80);
+
+ cmdbuf.append(executablePath);
+
+ for (int i = 1; i < cmd.length; ++i) {
+ cmdbuf.append(' ');
+ String s = cmd[i];
+ if (needsEscaping(isCmdFile, s)) {
+ cmdbuf.append('"');
+ cmdbuf.append(s);
+
+ // The code protects the [java.exe] and console command line
+ // parser, that interprets the [\"] combination as an escape
+ // sequence for the ["] char.
+ // http://msdn.microsoft.com/en-us/library/17w5ykft.aspx
+ //
+ // If the argument is an FS path, doubling of the tail [\]
+ // char is not a problem for non-console applications.
+ //
+ // The [\"] sequence is not an escape sequence for the [cmd.exe]
+ // command line parser. The case of the [""] tail escape
+ // sequence could not be realized due to the argument validation
+ // procedure.
+ if (!isCmdFile && s.endsWith("\\")) {
+ cmdbuf.append('\\');
+ }
+ cmdbuf.append('"');
+ } else {
+ cmdbuf.append(s);
+ }
+ }
+ return cmdbuf.toString();
+ }
+
// We guarantee the only command file execution for implicit [cmd.exe] run.
// http://technet.microsoft.com/en-us/library/bb490954.aspx
private static final char CMD_BAT_ESCAPE[] = {' ', '\t', '<', '>', '&', '|', '^'};
@@ -227,64 +290,89 @@
}
+ private boolean isShellFile(String executablePath) {
+ String upPath = executablePath.toUpperCase();
+ return (upPath.endsWith(".CMD") || upPath.endsWith(".BAT"));
+ }
+
+ private String quoteString(String arg) {
+ StringBuilder argbuf = new StringBuilder(arg.length() + 2);
+ return argbuf.append('"').append(arg).append('"').toString();
+ }
+
+
private long handle = 0;
private OutputStream stdin_stream;
private InputStream stdout_stream;
private InputStream stderr_stream;
- private ProcessImpl(final String cmd[],
+ private ProcessImpl(String cmd[],
final String envblock,
final String path,
final long[] stdHandles,
final boolean redirectErrorStream)
throws IOException
{
- // The [executablePath] is not quoted for any case.
- String executablePath = getExecutablePath(cmd[0]);
+ String cmdstr;
+ SecurityManager security = System.getSecurityManager();
+ boolean allowAmbigousCommands = false;
+ if (security == null) {
+ String value = System.getProperty("jdk.lang.Process.allowAmbigousCommands");
+ if (value != null)
+ allowAmbigousCommands = !"false".equalsIgnoreCase(value);
+ }
+ if (allowAmbigousCommands) {
+ // Legacy mode.
- // We need to extend the argument verification procedure
- // to guarantee the only command file execution for implicit [cmd.exe]
- // run.
- String upPath = executablePath.toUpperCase();
- boolean isCmdFile = (upPath.endsWith(".CMD") || upPath.endsWith(".BAT"));
+ // Normalize path if possible.
+ String executablePath = new File(cmd[0]).getPath();
- StringBuilder cmdbuf = new StringBuilder(80);
+ // No worry about internal and unpaired ["] .
+ if (needsEscaping(false, executablePath) )
+ executablePath = quoteString(executablePath);
- // Quotation protects from interpretation of the [path] argument as
- // start of longer path with spaces. Quotation has no influence to
- // [.exe] extension heuristic.
- cmdbuf.append('"');
- cmdbuf.append(executablePath);
- cmdbuf.append('"');
+ cmdstr = createCommandLine(
+ false, //legacy mode doesn't worry about extended verification
+ executablePath,
+ cmd);
+ } else {
+ String executablePath;
+ try {
+ executablePath = getExecutablePath(cmd[0]);
+ } catch (IllegalArgumentException e) {
+ // Workaround for the calls like
+ // Runtime.getRuntime().exec("\"C:\\Program Files\\foo\" bar")
- for (int i = 1; i < cmd.length; i++) {
- cmdbuf.append(' ');
- String s = cmd[i];
- if (needsEscaping(isCmdFile, s)) {
- cmdbuf.append('"');
- cmdbuf.append(s);
+ // No chance to avoid CMD/BAT injection, except to do the work
+ // right from the beginning. Otherwise we have too many corner
+ // cases from
+ // Runtime.getRuntime().exec(String[] cmd [, ...])
+ // calls with internal ["] and escape sequences.
+
+ // Restore original command line.
+ StringBuilder join = new StringBuilder();
+ // terminal space in command line is ok
+ for (String s : cmd)
+ join.append(s).append(' ');
- // The code protects the [java.exe] and console command line
- // parser, that interprets the [\"] combination as an escape
- // sequence for the ["] char.
- // http://msdn.microsoft.com/en-us/library/17w5ykft.aspx
- //
- // If the argument is an FS path, doubling of the tail [\]
- // char is not a problem for non-console applications.
- //
- // The [\"] sequence is not an escape sequence for the [cmd.exe]
- // command line parser. The case of the [""] tail escape
- // sequence could not be realized due to the argument validation
- // procedure.
- if (!isCmdFile && s.endsWith("\\")) {
- cmdbuf.append('\\');
- }
- cmdbuf.append('"');
- } else {
- cmdbuf.append(s);
+ // Parse the command line again.
+ cmd = getTokensFromCommand(join.toString());
+ executablePath = getExecutablePath(cmd[0]);
+
+ // Check new executable name once more
+ if (security != null)
+ security.checkExec(executablePath);
}
+
+ // Quotation protects from interpretation of the [path] argument as
+ // start of longer path with spaces. Quotation has no influence to
+ // [.exe] extension heuristic.
+ cmdstr = createCommandLine(
+ // We need the extended verification procedure for CMD files.
+ isShellFile(executablePath),
+ quoteString(executablePath),
+ cmd);
}
- String cmdstr = cmdbuf.toString();
handle = create(cmdstr, envblock, path,
stdHandles, redirectErrorStream);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/lang/Runtime/exec/ExecCommand.java Tue May 14 20:16:21 2013 +0400
@@ -0,0 +1,163 @@
+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+
+/**
+ * @test
+ * @bug 8012453
+ * @run main/othervm ExecCommand
+ * @summary workaround for legacy applications with Runtime.getRuntime().exec(String command)
+ */
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.security.AccessControlException;
+
+public class ExecCommand {
+ static class SecurityMan extends SecurityManager {
+ public static String unquote(String str)
+ {
+ int length = (str == null)
+ ? 0
+ : str.length();
+
+ if (length > 1
+ && str.charAt(0) == '\"'
+ && str.charAt(length - 1) == '\"')
+ {
+ return str.substring(1, length - 1);
+ }
+ return str;
+ }
+
+ @Override public void checkExec(String cmd) {
+ String ncmd = (new File(unquote(cmd))).getPath();
+ if ( ncmd.equals(".\\Program")
+ || ncmd.equals("\".\\Program")
+ || ncmd.equals(".\\Program Files\\do.cmd")
+ || ncmd.equals(".\\Program.cmd"))
+ {
+ return;
+ }
+ super.checkExec(cmd);
+ }
+ }
+
+ // Parameters for the Runtime.exec calls
+ private static final String TEST_RTE_ARG[] = {
+ ".\\Program Files\\do.cmd",
+ "\".\\Program Files\\doNot.cmd\" arg",
+ "\".\\Program Files\\do.cmd\" arg",
+ // compatibility
+ "\".\\Program.cmd\" arg",
+ ".\\Program.cmd arg",
+ };
+
+ private static final String doCmdCopy[] = {
+ ".\\Program.cmd",
+ ".\\Program Files\\doNot.cmd",
+ ".\\Program Files\\do.cmd",
+ };
+
+ // Golden image for results
+ private static final String TEST_RTE_GI[][] = {
+ //Pure system | Legacy mode | Legacy mode & SM
+ // [.\Program File\do.cmd]
+ new String[]{"IOException", // [.\Program] not found
+ "Success",
+ "IOException"}, //SM - no legacy mode [.\Program] - OK
+
+ // [".\Program File\doNot.cmd" arg]
+ new String[]{"Success",
+ "Success",
+ "AccessControlException"}, //SM - [".\Program] - OK,
+ // [.\\Program Files\\doNot.cmd] - Fail
+
+ // [".\Program File\do.cmd" arg]
+ // AccessControlException
+ new String[]{"Success",
+ "Success",
+ "Success"}, //SM - [".\Program] - OK,
+ // [.\\Program Files\\do.cmd] - OK
+
+ // compatibility
+ new String[]{"Success", "Success", "Success"}, //[".\Program.cmd"]
+ new String[]{"Success", "Success", "Success"} //[.\Program.cmd]
+ };
+
+ public static void main(String[] _args) throws Exception {
+ if (!System.getProperty("os.name").startsWith("Windows")) {
+ return;
+ }
+
+ // tear up
+ try {
+ new File(".\\Program Files").mkdirs();
+ for (int i = 0; i < doCmdCopy.length; ++i) {
+ try (BufferedWriter outCmd = new BufferedWriter(
+ new FileWriter(doCmdCopy[i]))) {
+ outCmd.write("@echo %1");
+ }
+ }
+ } catch (IOException e) {
+ throw new Error(e.getMessage());
+ }
+
+ // action
+ for (int k = 0; k < 3; ++k) {
+ switch (k) {
+ case 1:
+ System.setProperty("jdk.lang.Process.allowAmbigousCommands", "");
+ break;
+ case 2:
+ System.setSecurityManager( new SecurityMan() );
+ break;
+ }
+ for (int i = 0; i < TEST_RTE_ARG.length; ++i) {
+ String outRes;
+ try {
+ Process exec = Runtime.getRuntime().exec(TEST_RTE_ARG[i]);
+ exec.waitFor();
+ outRes = "Success";
+ } catch (IOException ioe) {
+ outRes = "IOException: " + ioe.getMessage();
+ } catch (IllegalArgumentException iae) {
+ outRes = "IllegalArgumentException: " + iae.getMessage();
+ } catch (AccessControlException se) {
+ outRes = "AccessControlException: " + se.getMessage();
+ }
+
+ if (!outRes.startsWith(TEST_RTE_GI[i][k])) {
+ throw new Error("Unexpected output! Step" + k + "" + i
+ + " \nArgument: " + TEST_RTE_ARG[i]
+ + "\nExpected: " + TEST_RTE_GI[i][k]
+ + "\n Output: " + outRes);
+ } else {
+ System.out.println("RTE OK:" + TEST_RTE_ARG[i]);
+ }
+ }
+ }
+ }
+}