Issue #10055 - Ensure that strong quoting is done only when needed, a…

…nd escaping is done properly.

Signed-off-by: Joakim Erdfelt <[email protected]>

jetty-home/src/main/resources/bin/jetty.sh

@@ -499,7 +499,8 @@ case "$ACTION" in
           disown \$!
           echo \$! > \"$JETTY_PID\""
       else
-        echo ${RUN_ARGS[*]} | xargs ${JAVA} > /dev/null &
+        # echo ${RUN_ARGS[*]} | xargs ${JAVA} > /dev/null &
+        echo ${RUN_ARGS[*]} | xargs ${JAVA} & # TODO: don't check in this version only used for cli debugging
         disown $!
         echo $! > "$JETTY_PID"
       fi

jetty-start/src/main/java/org/eclipse/jetty/start/CommandLineBuilder.java

@@ -15,6 +15,7 @@
 
 import java.io.File;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 public class CommandLineBuilder
@@ -67,6 +68,26 @@ public static String quote(String arg)
         return "'" + arg + "'";
     }
 
+    /**
+     * Map of ASCII characters that indicate a need for quoting
+     */
+    private static final boolean[] NEEDS_QUOTING;
+
+    static
+    {
+        NEEDS_QUOTING = new boolean[256];
+        Arrays.fill(NEEDS_QUOTING, true);
+        // Allow certain characters (that shouldn't trigger quoting)
+        for (int i = 'a'; i < 'z'; i++)
+            NEEDS_QUOTING[i] = false;
+        for (int i = 'A'; i < 'Z'; i++)
+            NEEDS_QUOTING[i] = false;
+        for (int i = '0'; i < '9'; i++)
+            NEEDS_QUOTING[i] = false;
+        for (char c : "-./=_:\\".toCharArray())
+            NEEDS_QUOTING[c] = false;
+    }
+
     private List<String> args;
 
     public CommandLineBuilder()
@@ -165,9 +186,9 @@ public String toString(String delim)
     }
 
     /**
-     * A version of {@link #toString()} where every arg is evaluated for potential {@code '} (single-quote tick) wrapping.
+     * A version of {@link #toString()} where every arg is evaluated for potential {@code '} (strong quote) wrapping.
      *
-     * @return the toString but each arg that has spaces is surrounded by {@code '} (single-quote tick)
+     * @return the toString but each arg that has spaces is surrounded by {@code '} (string quote)
      */
     public String toQuotedString()
     {
@@ -177,17 +198,66 @@ public String toQuotedString()
         {
             if (buf.length() > 0)
                 buf.append(' ');
-            boolean needsQuotes = (arg.contains(" "));
-            if (needsQuotes)
-                buf.append("'");
-            buf.append(arg);
-            if (needsQuotes)
-                buf.append("'");
+            shellQuoteIfNeeded(buf, arg);
         }
 
         return buf.toString();
     }
 
+    private static boolean needsQuoting(String input)
+    {
+        for (int i = 0; i < input.length(); i++)
+        {
+            char c = input.charAt(i);
+
+            // Characters that require quoting
+            if (NEEDS_QUOTING[c])
+            {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Quote a string suitable for use with a command line shell using strong quotes {@code '}.
+     * <p>This method applies strong quoting {@code '} as described for the unix {@code sh} commands:
+     * Enclosing characters within strong quotes preserves the literal meaning of all characters.
+     *
+     * @param buf the StringBuilder to append to
+     * @param input The string to quote if needed
+     */
+    public static void shellQuoteIfNeeded(StringBuilder buf, String input)
+    {
+        if (input == null)
+            return;
+
+        if (input.length() == 0)
+        {
+            buf.append("''");
+            return;
+        }
+
+        if (!needsQuoting(input))
+        {
+            buf.append(input);
+            return;
+        }
+
+        buf.append('\'');
+
+        for (int i = 0; i < input.length(); i++)
+        {
+            char c = input.charAt(i);
+            if (c == '\'')
+                buf.append("'\\''");
+            else
+                buf.append(c);
+        }
+
+        buf.append('\'');
+    }
+
     public void debug()
     {
         if (!StartLog.isDebugEnabled())

jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java

@@ -819,7 +819,7 @@ public CommandLineBuilder getMainArgs(Set<String> parts) throws IOException
             {
                 for (Prop p : properties)
                 {
-                    cmd.addRawArg(CommandLineBuilder.quote(p.key) + "=" + CommandLineBuilder.quote(properties.expand(p.value)));
+                    cmd.addRawArg(p.key + "=" + properties.expand(p.value));
                 }
             }
             else if (properties.size() > 0)

jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt

@@ -60,12 +60,13 @@ Report Commands:
 
   --dry-run
                    Prints the command line that start.jar generates,
-                   then exits.
+                   in a format usable by the unix sh command, then exits.
                    This may be used to generate command lines into scripts:
                      $ java -jar start.jar --dry-run > jetty.sh
 
   --dry-run=<part>(,<part>)*
-                   Prints specific parts of the command line. The parts are:
+                   Prints specific parts of the command line in a format usable by
+                   the unix sh command. The parts are:
                      o  "java" - the JVM to run
                      o  "opts" - the JVM options (e.g. -D, -X and -XX flags)
                      o  "path" - the JVM class-path and/or the JPMS module-path

jetty-start/src/test/java/org/eclipse/jetty/start/CommandLineBuilderTest.java

@@ -13,13 +13,43 @@
 
 package org.eclipse.jetty.start;
 
+import java.util.stream.Stream;
+
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 
 public class CommandLineBuilderTest
 {
+    public static Stream<Arguments> shellQuoteIfNeededSource()
+    {
+        return Stream.of(
+            // The following consists of raw arguments from the start mechanism
+            // compared to the output if quoted for shell usage (--dry-run)
+            Arguments.of("", "''"),
+            Arguments.of("Foo", "Foo"),
+            Arguments.of("Foo,Bar", "'Foo,Bar'"),
+            Arguments.of("Foo Bar", "'Foo Bar'"),
+            Arguments.of("An 'internal' quoting", "'An '\\''internal'\\'' quoting'"),
+            Arguments.of("requestlog.format=%u cost for $USER", "'requestlog.format=%u cost for $USER'"),
+            // Raw system property, as defined in start.d/exec.ini
+            Arguments.of("-Dxxx.key=more values", "'-Dxxx.key=more values'")
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("shellQuoteIfNeededSource")
+    public void testNeedsShellQuoting(String input, String expected)
+    {
+        StringBuilder result = new StringBuilder();
+        CommandLineBuilder.shellQuoteIfNeeded(result, input);
+        assertThat(result.toString(), is(expected));
+    }
+
     @Test
     public void testSimpleCommandline()
     {