8012453: (process) Runtime.exec(String) fails if command contains spaces [win]

Reviewed-by: alanb

src/share/classes/java/lang/ProcessBuilder.java

@@ -29,7 +29,6 @@ import java.io.File;
 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 @@ public final class ProcessBuilder
                                      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);

src/windows/classes/java/lang/ProcessImpl.java

@@ -37,7 +37,10 @@ import java.io.BufferedOutputStream;
 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 @@ final class ProcessImpl extends Process {
 
     }
 
+    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 @@ final class ProcessImpl extends Process {
     }
 
 
+    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]);
-
-        // 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"));
-
-        StringBuilder cmdbuf = new StringBuilder(80);
+        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.
 
-        // 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('"');
+            // Normalize path if possible.
+            String executablePath = new File(cmd[0]).getPath();
 
-        for (int i = 1; i < cmd.length; i++) {
-            cmdbuf.append(' ');
-            String s = cmd[i];
-            if (needsEscaping(isCmdFile, s)) {
-                cmdbuf.append('"');
-                cmdbuf.append(s);
+            // No worry about internal and unpaired ["] .
+            if (needsEscaping(false, executablePath) )
+                executablePath = quoteString(executablePath);
 
-                // 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);
+            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")
+
+                // 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(' ');
+
+                // 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);

test/java/lang/Runtime/exec/ExecCommand.java

+/*
+ * 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]);
+                }
+            }
+        }
+    }
+}