CmdShell: quote items that cmd.exe would otherwise interpret

src/main/java/org/apache/maven/shared/utils/cli/shell/CmdShell.java

@@ -27,6 +27,12 @@
  * @author <a href="mailto:carlos@apache.org">Carlos Sanchez</a>
  */
 public class CmdShell extends Shell {
+    /**
+     * Characters that make {@code cmd.exe} interpret an unquoted item: whitespace and, per {@code cmd.exe /?},
+     * {@code &<>()@^|}.
+     */
+    private static final char[] CMD_SPECIAL_CHARS = {' ', '\t', '&', '<', '>', '(', ')', '@', '^', '|'};
+
     /**
      * Create an instance of CmdShell.
      */
@@ -76,6 +82,34 @@ public CmdShell() {
      * @param arguments the arguments for the executable
      * @return the resulting command line
      */
+    /**
+     * Quotes an item that contains a character {@code cmd.exe} would otherwise interpret: whitespace and the
+     * special characters listed above, or every item when {@link #isUnconditionalQuoting()} is set. An item that
+     * is already surrounded by double quotes is left as it is.
+     *
+     * @param inputString the executable or argument
+     * @param isExecutable unused, the executable and the arguments are quoted the same way
+     * @return the item, surrounded by double quotes when {@code cmd.exe} needs them
+     */
+    @Override
+    protected String quoteOneItem(String inputString, boolean isExecutable) {
+        if (inputString == null || inputString.isEmpty()) {
+            return inputString;
+        }
+        if (inputString.length() > 1 && inputString.startsWith("\"") && inputString.endsWith("\"")) {
+            return inputString;
+        }
+        if (isUnconditionalQuoting()) {
+            return "\"" + inputString + "\"";
+        }
+        for (char c : CMD_SPECIAL_CHARS) {
+            if (inputString.indexOf(c) >= 0) {
+                return "\"" + inputString + "\"";
+            }
+        }
+        return inputString;
+    }
+
     @Override
     public List<String> getCommandLine(String executable, String... arguments) {
         StringBuilder sb = new StringBuilder();

src/main/java/org/apache/maven/shared/utils/cli/shell/Shell.java

@@ -35,8 +35,6 @@
  * @author <a href="mailto:carlos@apache.org">Carlos Sanchez</a>
  */
 public class Shell {
-    private static final char[] DEFAULT_QUOTING_TRIGGER_CHARS = {' '};
-
     private String shellCommand;
 
     private final List<String> shellArgs = new ArrayList<>();
@@ -142,10 +140,6 @@ List<String> getRawCommandLine(String executableParameter, String... argumentsPa
         return commandLine;
     }
 
-    char[] getQuotingTriggerChars() {
-        return DEFAULT_QUOTING_TRIGGER_CHARS;
-    }
-
     String getExecutionPreamble() {
         return null;
     }

src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java

@@ -18,7 +18,9 @@
  */
 package org.apache.maven.shared.utils.cli;
 
+import java.io.File;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Locale;
@@ -27,6 +29,7 @@
 
 import org.apache.maven.shared.utils.Os;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -144,6 +147,51 @@ public void executeCommandLineWithLargeStdoutCompletesWithoutFailure() throws Ex
         }
     }
 
+    @TempDir
+    File tempDir;
+
+    @Test
+    public void executeCommandLineOnWindowsPassesCmdSpecialCharactersThrough() throws Exception {
+        if (!Os.isFamily(Os.FAMILY_WINDOWS)) {
+            return;
+        }
+
+        // MSHARED-765: unquoted, cmd.exe runs "echo a" and then tries to run "b"
+        Commandline cl = new Commandline();
+        cl.setExecutable("echo");
+        cl.createArg().setValue("a&b");
+
+        CommandLineUtils.StringStreamConsumer stdout = new CommandLineUtils.StringStreamConsumer();
+        CommandLineUtils.StringStreamConsumer stderr = new CommandLineUtils.StringStreamConsumer();
+        int exitCode = CommandLineUtils.executeCommandLine(cl, stdout, stderr);
+
+        assertEquals(0, exitCode, stderr.getOutput());
+        assertEquals("\"a&b\"" + System.lineSeparator(), stdout.getOutput());
+    }
+
+    @Test
+    public void executeCommandLineOnWindowsRunsExecutableFromPathWithParentheses() throws Exception {
+        if (!Os.isFamily(Os.FAMILY_WINDOWS)) {
+            return;
+        }
+
+        // MSHARED-832: C:\work\lol(1)\maven\bin\mvn.cmd
+        File dir = new File(tempDir, "lol(1)");
+        assertTrue(dir.mkdirs());
+        File script = new File(dir, "x.cmd");
+        Files.write(script.toPath(), "@echo ok".getBytes(StandardCharsets.US_ASCII));
+
+        Commandline cl = new Commandline();
+        cl.setExecutable(script.getAbsolutePath());
+
+        CommandLineUtils.StringStreamConsumer stdout = new CommandLineUtils.StringStreamConsumer();
+        CommandLineUtils.StringStreamConsumer stderr = new CommandLineUtils.StringStreamConsumer();
+        int exitCode = CommandLineUtils.executeCommandLine(cl, stdout, stderr);
+
+        assertEquals(0, exitCode, stderr.getOutput());
+        assertEquals("ok" + System.lineSeparator(), stdout.getOutput());
+    }
+
     @Test
     public void executeCommandLineDecodesOutputWithTheGivenCharset() throws Exception {
         if (!Os.isFamily(Os.FAMILY_UNIX)) {

src/test/java/org/apache/maven/shared/utils/cli/shell/CmdShellTest.java

@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.shared.utils.cli.shell;
+
+import java.util.List;
+
+import org.apache.maven.shared.utils.cli.Commandline;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * The command line built here is a string, so these tests run on every platform. {@code Commandline} rewrites the
+ * path separators of the executable for the current platform, so the expected strings are built from
+ * {@code getExecutable()} rather than from the literal path.
+ */
+public class CmdShellTest {
+
+    private static Commandline commandline(String executable, String... arguments) {
+        Commandline commandline = new Commandline(new CmdShell());
+        commandline.setExecutable(executable);
+        for (String argument : arguments) {
+            commandline.createArg().setValue(argument);
+        }
+        return commandline;
+    }
+
+    private static String commandLine(Commandline commandline) {
+        List<String> lines = commandline.getShell().getShellCommandLine(commandline.getArguments());
+        assertEquals("cmd.exe", lines.get(0));
+        assertEquals("/X", lines.get(1));
+        assertEquals("/C", lines.get(2));
+        assertEquals(4, lines.size());
+        return lines.get(3);
+    }
+
+    @Test
+    public void plainItemsAreNotQuoted() {
+        assertEquals("\"mvn.cmd -B compile\"", commandLine(commandline("mvn.cmd", "-B", "compile")));
+    }
+
+    @Test
+    public void executableWithParenthesesIsQuoted() {
+        // MSHARED-832: C:\work\lol(1)\maven\bin\mvn.cmd
+        Commandline cl = commandline("C:\\work\\lol(1)\\maven\\bin\\mvn.cmd", "-B", "compile");
+        assertEquals("\"\"" + cl.getExecutable() + "\" -B compile\"", commandLine(cl));
+    }
+
+    @Test
+    public void executableWithSpaceIsQuoted() {
+        Commandline cl = commandline("C:\\Program Files\\maven\\bin\\mvn.cmd", "-B");
+        assertEquals("\"\"" + cl.getExecutable() + "\" -B\"", commandLine(cl));
+    }
+
+    @Test
+    public void argumentWithCmdSpecialCharactersIsQuoted() {
+        // MSHARED-765: a password containing &
+        assertEquals("\"jarsigner -storepass \"a&b\"\"", commandLine(commandline("jarsigner", "-storepass", "a&b")));
+        assertEquals(
+                "\"x \"a|b\" \"c<d\" \"e>f\" \"g^h\" \"i@j\" \"k l\"\"",
+                commandLine(commandline("x", "a|b", "c<d", "e>f", "g^h", "i@j", "k l")));
+    }
+
+    @Test
+    public void alreadyQuotedItemIsLeftAlone() {
+        assertEquals("\"x \"a b\"\"", commandLine(commandline("x", "\"a b\"")));
+    }
+
+    @Test
+    public void unconditionalQuotingQuotesEveryItem() {
+        Commandline cl = commandline("x", "plain");
+        cl.getShell().setUnconditionalQuoting(true);
+        assertEquals("\"\"x\" \"plain\"\"", commandLine(cl));
+    }
+
+    @Test
+    public void quotingCanBeDisabled() {
+        Commandline cl = commandline("x", "a&b");
+        cl.getShell().setQuotedArgumentsEnabled(false);
+        assertEquals("\"x a&b\"", commandLine(cl));
+    }
+}