From 11d2a32eea2c89a2ef42620081fc7dfd179b80fc Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 14 Jul 2023 17:05:41 +0200 Subject: [PATCH] Fixes #9999 and #10055 - proper quoting for dry-run Using "strong quoting" with the single quote character to quote argument of --dry-run output. Signed-off-by: gregw Signed-off-by: Simone Bordet Co-authored-by: Simone Bordet --- .../jetty/start/CommandLineBuilder.java | 213 +++++++++++++----- .../java/org/eclipse/jetty/start/Main.java | 2 +- .../org/eclipse/jetty/start/StartArgs.java | 68 +++--- .../org/eclipse/jetty/start/usage.txt | 5 +- .../jetty/start/CommandLineBuilderTest.java | 86 +++++-- .../org/eclipse/jetty/start/MainTest.java | 2 +- 6 files changed, 270 insertions(+), 106 deletions(-) diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/CommandLineBuilder.java b/jetty-start/src/main/java/org/eclipse/jetty/start/CommandLineBuilder.java index 7020159ec1f..ca1f9f900b7 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/CommandLineBuilder.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/CommandLineBuilder.java @@ -16,6 +16,7 @@ package org.eclipse.jetty.start; import java.io.File; import java.util.ArrayList; import java.util.List; +import java.util.Objects; public class CommandLineBuilder { @@ -54,12 +55,9 @@ public class CommandLineBuilder } /** - * Perform an optional quoting of the argument, being intelligent with spaces and quotes as needed. If a subString is set in quotes it won't the subString - * won't be escaped. - * - * @param arg the argument to quote - * @return the quoted and escaped argument - * @deprecated no replacement, quoting is done by {@link #toQuotedString()} now. + * @param arg string + * @return Quoted string + * @deprecated use {@link #shellQuoteIfNeeded(String)} */ @Deprecated public static String quote(String arg) @@ -67,23 +65,97 @@ public class CommandLineBuilder return "'" + arg + "'"; } - private List args; + private final StringBuilder commandLine = new StringBuilder(); + private final List args = new ArrayList<>(); + private final String separator; public CommandLineBuilder() { - args = new ArrayList(); + this(false); } + @Deprecated public CommandLineBuilder(String bin) { this(); args.add(bin); } + public CommandLineBuilder(boolean multiline) + { + separator = multiline ? (" \\" + System.lineSeparator() + " ") : " "; + } + /** - * Add a simple argument to the command line. - *

- * Will quote arguments that have a space in them. + * This method applies single quotes suitable for a POSIX compliant shell if + * necessary. + * + * @param input The string to quote if needed + * @return The quoted string or the original string if quotes are not necessary + */ + public static String shellQuoteIfNeeded(String input) + { + // Single quotes are used because double quotes + // are evaluated differently by some shells. + + if (input == null) + return null; + if (input.length() == 0) + return "''"; + + int i = 0; + boolean needsQuoting = false; + while (!needsQuoting && i < input.length()) + { + char c = input.charAt(i++); + + // needs quoting unless a limited set of known good characters + needsQuoting = !( + (c >= 'A' && c <= 'Z') || + (c >= 'a' && c <= 'z') || + (c >= '0' && c <= '9') || + c == '/' || + c == ':' || + c == '.' || + c == ',' || + c == '+' || + c == '-' || + c == '_' + ); + } + + if (!needsQuoting) + return input; + + StringBuilder builder = new StringBuilder(input.length() * 2); + builder.append("'"); + builder.append(input, 0, --i); + + while (i < input.length()) + { + char c = input.charAt(i++); + if (c == '\'') + { + // There is no escape for a literal single quote, so we must leave the quotes + // and then escape the single quote. We test for the start/end of the string, so + // we can be less ugly in those cases. + if (i == 1) + builder.insert(0, "\\").append("'"); + else if (i == input.length()) + builder.append("'\\"); + else + builder.append("'\\''"); + } + else + builder.append(c); + } + builder.append("'"); + + return builder.toString(); + } + + /** + * Add a simple argument to the command line, quoted if necessary. * * @param arg the simple argument to add */ @@ -91,49 +163,93 @@ public class CommandLineBuilder { if (arg != null) { + if (commandLine.length() > 0) + commandLine.append(separator); args.add(arg); + commandLine.append(shellQuoteIfNeeded(arg)); } } /** - * Similar to {@link #addArg(String)} but concats both name + value with an "=" sign, quoting were needed, and excluding the "=" portion if the value is - * undefined or empty. - * - * 

-     *   addEqualsArg("-Dname", "value") = "-Dname=value"
-     *   addEqualsArg("-Djetty.home", "/opt/company inc/jetty (7)/") = "-Djetty.home=/opt/company\ inc/jetty\ (7)/"
-     *   addEqualsArg("-Djenkins.workspace", "/opt/workspaces/jetty jdk7/") = "-Djenkins.workspace=/opt/workspaces/jetty\ jdk7/"
-     *   addEqualsArg("-Dstress", null) = "-Dstress"
-     *   addEqualsArg("-Dstress", "") = "-Dstress"
-     *
- * + * @deprecated use {@link #addArg(String, String)} + */ + @Deprecated + public void addEqualsArg(String name, String value) + { + addArg(name, value); + } + + /** + * Add a "name=value" style argument to the command line with + * name and value quoted if necessary. * @param name the name * @param value the value */ - public void addEqualsArg(String name, String value) + public void addArg(String name, String value) { + Objects.requireNonNull(name); + + if (commandLine.length() > 0) + commandLine.append(separator); + if ((value != null) && (value.length() > 0)) { args.add(name + "=" + value); + commandLine.append(shellQuoteIfNeeded(name)).append('=').append(shellQuoteIfNeeded(value)); } else { args.add(name); + commandLine.append(shellQuoteIfNeeded(name)); } } /** - * Add a simple argument to the command line. - *

- * Will NOT quote/escape arguments that have a space in them. - * - * @param arg the simple argument to add + * @deprecated use {@link #addArg(String)} */ + @Deprecated public void addRawArg(String arg) { - if (arg != null) + addArg(arg); + } + + /** + * Adds a "-OPTION" style option to the command line with no quoting, for example `--help`. + * @param option the option + */ + public void addOption(String option) + { + addOption(option, null, null); + } + + /** + * Adds a "-OPTIONname=value" style option to the command line with + * name and value quoted if necessary, for example "-Dprop=value". + * @param option the option + * @param name the name + * @param value the value + */ + public void addOption(String option, String name, String value) + { + Objects.requireNonNull(option); + + if (commandLine.length() > 0) + commandLine.append(separator); + + if (name == null || name.length() == 0) { - args.add(arg); + commandLine.append(option); + args.add(option); + } + else if ((value != null) && (value.length() > 0)) + { + args.add(option + name + "=" + value); + commandLine.append(option).append(shellQuoteIfNeeded(name)).append('=').append(shellQuoteIfNeeded(value)); + } + else + { + args.add(option + name); + commandLine.append(option).append(shellQuoteIfNeeded(name)); } } @@ -144,48 +260,35 @@ public class CommandLineBuilder @Override public String toString() - { - return toString(" "); - } - - public String toString(String delim) { StringBuilder buf = new StringBuilder(); - for (String arg : args) { if (buf.length() > 0) - { - buf.append(delim); - } + buf.append(' '); buf.append(arg); // we assume escaping has occurred during addArg } return buf.toString(); } + /** + * @deprecated use {@link #toCommandLine()} + */ + @Deprecated + public String toQuotedString() + { + return toCommandLine(); + } + /** * A version of {@link #toString()} where every arg is evaluated for potential {@code '} (single-quote tick) wrapping. * * @return the toString but each arg that has spaces is surrounded by {@code '} (single-quote tick) */ - public String toQuotedString() + public String toCommandLine() { - StringBuilder buf = new StringBuilder(); - - for (String arg : args) - { - if (buf.length() > 0) - buf.append(' '); - boolean needsQuotes = (arg.contains(" ")); - if (needsQuotes) - buf.append("'"); - buf.append(arg); - if (needsQuotes) - buf.append("'"); - } - - return buf.toString(); + return commandLine.toString(); } public void debug() diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java index 425805d8e50..08d991360aa 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java @@ -441,7 +441,7 @@ public class Main { CommandLineBuilder cmd = args.getMainArgs(args.getDryRunParts()); cmd.debug(); - System.out.println(cmd.toQuotedString()); + System.out.println(cmd.toCommandLine()); } if (args.isStopCommand()) diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java b/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java index 13b265ce5ec..b1554fa0237 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/StartArgs.java @@ -208,6 +208,7 @@ public class StartArgs private boolean listConfig = false; private boolean version = false; private boolean dryRun = false; + private boolean multiLine = false; private final Set dryRunParts = new HashSet<>(); private boolean jpms = false; private boolean createStartD = false; @@ -706,7 +707,7 @@ public class StartArgs if (parts.isEmpty()) parts = ALL_PARTS; - CommandLineBuilder cmd = new CommandLineBuilder(); + CommandLineBuilder cmd = new CommandLineBuilder(multiLine); // Special Stop/Shutdown properties ensureSystemPropertySet("STOP.PORT"); @@ -714,13 +715,13 @@ public class StartArgs ensureSystemPropertySet("STOP.WAIT"); if (parts.contains("java")) - cmd.addRawArg(CommandLineBuilder.findJavaBin()); + cmd.addArg(CommandLineBuilder.findJavaBin()); if (parts.contains("opts")) { - cmd.addRawArg("-Djava.io.tmpdir=" + System.getProperty("java.io.tmpdir")); - cmd.addRawArg("-Djetty.home=" + baseHome.getHome()); - cmd.addRawArg("-Djetty.base=" + baseHome.getBase()); + cmd.addOption("-D", "java.io.tmpdir", System.getProperty("java.io.tmpdir")); + cmd.addOption("-D", "jetty.home", baseHome.getHome()); + cmd.addOption("-D", "jetty.base", baseHome.getBase()); for (String x : getJvmArgSources().keySet()) { @@ -731,11 +732,11 @@ public class StartArgs String value = assign.length == 1 ? "" : assign[1]; Prop p = processSystemProperty(key, value, null); - cmd.addRawArg("-D" + p.key + "=" + getProperties().expand(p.value)); + cmd.addOption("-D", p.key, properties.expand(p.value)); } else { - cmd.addRawArg(getProperties().expand(x)); + cmd.addArg(getProperties().expand(x)); } } @@ -743,7 +744,7 @@ public class StartArgs for (String propKey : systemPropertySource.keySet()) { String value = System.getProperty(propKey); - cmd.addEqualsArg("-D" + propKey, value); + cmd.addOption("-D", propKey, value); } } @@ -756,60 +757,60 @@ public class StartArgs List files = dirsAndFiles.get(false); if (files != null && !files.isEmpty()) { - cmd.addRawArg("--module-path"); + cmd.addOption("--module-path"); String modules = files.stream() .map(File::getAbsolutePath) .collect(Collectors.joining(File.pathSeparator)); - cmd.addRawArg(modules); + cmd.addArg(modules); } List dirs = dirsAndFiles.get(true); if (dirs != null && !dirs.isEmpty()) { - cmd.addRawArg("--class-path"); + cmd.addOption("--class-path"); String directories = dirs.stream() .map(File::getAbsolutePath) .collect(Collectors.joining(File.pathSeparator)); - cmd.addRawArg(directories); + cmd.addArg(directories); } if (!jmodAdds.isEmpty()) { - cmd.addRawArg("--add-modules"); - cmd.addRawArg(String.join(",", jmodAdds)); + cmd.addOption("--add-modules"); + cmd.addArg(String.join(",", jmodAdds)); } for (Map.Entry> entry : jmodPatch.entrySet()) { - cmd.addRawArg("--patch-module"); - cmd.addRawArg(entry.getKey() + "=" + String.join(File.pathSeparator, entry.getValue())); + cmd.addOption("--patch-module"); + cmd.addArg(entry.getKey(), String.join(File.pathSeparator, entry.getValue())); } for (Map.Entry> entry : jmodOpens.entrySet()) { - cmd.addRawArg("--add-opens"); - cmd.addRawArg(entry.getKey() + "=" + String.join(",", entry.getValue())); + cmd.addOption("--add-opens"); + cmd.addArg(entry.getKey(), String.join(",", entry.getValue())); } for (Map.Entry> entry : jmodExports.entrySet()) { - cmd.addRawArg("--add-exports"); - cmd.addRawArg(entry.getKey() + "=" + String.join(",", entry.getValue())); + cmd.addOption("--add-exports"); + cmd.addArg(entry.getKey(), String.join(",", entry.getValue())); } for (Map.Entry> entry : jmodReads.entrySet()) { - cmd.addRawArg("--add-reads"); - cmd.addRawArg(entry.getKey() + "=" + String.join(",", entry.getValue())); + cmd.addOption("--add-reads"); + cmd.addArg(entry.getKey(), String.join(",", entry.getValue())); } } else { - cmd.addRawArg("--class-path"); - cmd.addRawArg(classpath.toString()); + cmd.addOption("--class-path"); + cmd.addArg(classpath.toString()); } } if (parts.contains("main")) { if (isJPMS()) - cmd.addRawArg("--module"); - cmd.addRawArg(getMainClassname()); + cmd.addOption("--module"); + cmd.addArg(getMainClassname()); } // pass properties as args or as a file @@ -819,7 +820,8 @@ public class StartArgs { for (Prop p : properties) { - cmd.addRawArg(CommandLineBuilder.quote(p.key) + "=" + CommandLineBuilder.quote(properties.expand(p.value))); + if (!p.key.startsWith("java.version.")) + cmd.addArg(p.key, properties.expand(p.value)); } } else if (properties.size() > 0) @@ -837,17 +839,17 @@ public class StartArgs { properties.store(out, "start.jar properties"); } - cmd.addRawArg(propPath.toAbsolutePath().toString()); + cmd.addArg(propPath.toAbsolutePath().toString()); } for (Path xml : xmls) { - cmd.addRawArg(xml.toAbsolutePath().toString()); + cmd.addArg(xml.toAbsolutePath().toString()); } for (Path propertyFile : propertyFiles) { - cmd.addRawArg(propertyFile.toAbsolutePath().toString()); + cmd.addArg(propertyFile.toAbsolutePath().toString()); } } @@ -1225,6 +1227,12 @@ public class StartArgs int colon = arg.indexOf('='); for (String part : arg.substring(colon + 1).split(",")) { + if ("multiline".equalsIgnoreCase(part)) + { + multiLine = true; + continue; + } + if (!ALL_PARTS.contains(part)) throw new UsageException(UsageException.ERR_BAD_ARG, "Unrecognized --dry-run=\"%s\" in %s", part, source); diff --git a/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt b/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt index 19a865b7ccd..5813c199fdd 100644 --- a/jetty-start/src/main/resources/org/eclipse/jetty/start/usage.txt +++ b/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 a POSIX compliant shell, then exits. This may be used to generate command lines into scripts: $ java -jar start.jar --dry-run > jetty.sh --dry-run=(,)* - Prints specific parts of the command line. The parts are: + Prints specific parts of the command line in a format usable by + a POSIX compliant shell. 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 diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/CommandLineBuilderTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/CommandLineBuilderTest.java index 06753229e7e..6f938f7ae00 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/CommandLineBuilderTest.java +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/CommandLineBuilderTest.java @@ -13,7 +13,12 @@ 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; @@ -23,43 +28,90 @@ public class CommandLineBuilderTest @Test public void testSimpleCommandline() { - CommandLineBuilder cmd = new CommandLineBuilder("java"); - cmd.addEqualsArg("-Djava.io.tmpdir", "/home/java/temp dir/"); + CommandLineBuilder cmd = new CommandLineBuilder(); + cmd.addArg("java"); + cmd.addArg("-Djava.io.tmpdir", "/home/java/temp dir/"); cmd.addArg("--version"); - assertThat(cmd.toQuotedString(), is("java '-Djava.io.tmpdir=/home/java/temp dir/' --version")); + assertThat(cmd.toCommandLine(), is("java -Djava.io.tmpdir='/home/java/temp dir/' --version")); } @Test public void testSimpleHomeNoSpace() { - CommandLineBuilder cmd = new CommandLineBuilder("java"); - cmd.addEqualsArg("-Djetty.home", "/opt/jetty"); - assertThat(cmd.toQuotedString(), is("java -Djetty.home=/opt/jetty")); + CommandLineBuilder cmd = new CommandLineBuilder(); + cmd.addArg("java"); + cmd.addArg("-Djetty.home", "/opt/jetty"); + assertThat(cmd.toCommandLine(), is("java -Djetty.home=/opt/jetty")); } @Test public void testSimpleHomeWithSpace() { - CommandLineBuilder cmd = new CommandLineBuilder("java"); - cmd.addEqualsArg("-Djetty.home", "/opt/jetty 10/home"); - assertThat(cmd.toQuotedString(), is("java '-Djetty.home=/opt/jetty 10/home'")); + CommandLineBuilder cmd = new CommandLineBuilder(); + cmd.addArg("java"); + cmd.addArg("-Djetty.home", "/opt/jetty 10/home"); + assertThat(cmd.toCommandLine(), is("java -Djetty.home='/opt/jetty 10/home'")); } @Test public void testEscapedFormattingString() { - CommandLineBuilder cmd = new CommandLineBuilder("java"); - cmd.addEqualsArg("-Djetty.home", "/opt/jetty"); - cmd.addEqualsArg("jetty.requestlog.formatter", "%{client}a - %u %{dd/MMM/yyyy:HH:mm:ss ZZZ|GMT}t \"%r\" %s %O \"%{Referer}i\" \"%{User-Agent}i\""); - assertThat(cmd.toQuotedString(), is("java -Djetty.home=/opt/jetty 'jetty.requestlog.formatter=%{client}a - %u %{dd/MMM/yyyy:HH:mm:ss ZZZ|GMT}t \"%r\" %s %O \"%{Referer}i\" \"%{User-Agent}i\"'")); + CommandLineBuilder cmd = new CommandLineBuilder(); + cmd.addArg("java"); + cmd.addArg("-Djetty.home", "/opt/jetty"); + cmd.addArg("jetty.requestlog.formatter", "%{client}a - %u %{dd/MMM/yyyy:HH:mm:ss ZZZ|GMT}t \"%r\" %s %O \"%{Referer}i\" \"%{User-Agent}i\""); + assertThat(cmd.toCommandLine(), is("java -Djetty.home=/opt/jetty jetty.requestlog.formatter='%{client}a - %u %{dd/MMM/yyyy:HH:mm:ss ZZZ|GMT}t \"%r\" %s %O \"%{Referer}i\" \"%{User-Agent}i\"'")); } @Test public void testEscapeUnicode() { - CommandLineBuilder cmd = new CommandLineBuilder("java"); - cmd.addEqualsArg("-Djetty.home", "/opt/jetty"); - cmd.addEqualsArg("monetary.symbol", "€"); - assertThat(cmd.toQuotedString(), is("java -Djetty.home=/opt/jetty monetary.symbol=€")); + CommandLineBuilder cmd = new CommandLineBuilder(); + cmd.addArg("java"); + cmd.addArg("-Djetty.home", "/opt/jetty"); + cmd.addArg("monetary.symbol", "€"); + assertThat(cmd.toCommandLine(), is("java -Djetty.home=/opt/jetty monetary.symbol='€'")); + } + + public static Stream shellQuoting() + { + return Stream.of( + Arguments.of(null, null), + Arguments.of("", "''"), + Arguments.of("Hello", "Hello"), + Arguments.of("Hell0", "Hell0"), + Arguments.of("Hello$World", "'Hello$World'"), + Arguments.of("Hello\\World", "'Hello\\World'"), + Arguments.of("Hello`World", "'Hello`World'"), + Arguments.of("'Hello World'", "\\''Hello World'\\'"), + Arguments.of("\"Hello World\"", "'\"Hello World\"'"), + Arguments.of("H-llo_world", "H-llo_world"), + Arguments.of("H:llo/world", "H:llo/world"), + Arguments.of("Hello World", "'Hello World'"), + Arguments.of("foo\\bar", "'foo\\bar'"), + Arguments.of("foo'bar", "'foo'\\''bar'"), + Arguments.of("some 'internal' quoting", "'some '\\''internal'\\'' quoting'"), + Arguments.of("monetary.symbol=€", "'monetary.symbol=€'") + ); + } + + @ParameterizedTest + @MethodSource("shellQuoting") + public void testShellQuoting(String string, String expected) + { + assertThat(CommandLineBuilder.shellQuoteIfNeeded(string), is(expected)); + } + + @Test + public void testMultiLine() + { + CommandLineBuilder cmd = new CommandLineBuilder(true); + cmd.addArg("java"); + cmd.addArg("-Djetty.home", "/opt/jetty"); + cmd.addArg("monetary.symbol", "€"); + assertThat(cmd.toCommandLine(), + is("java \\" + System.lineSeparator() + + " -Djetty.home=/opt/jetty \\" + System.lineSeparator() + + " monetary.symbol='€'")); } } diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java index 94a3ddafa5b..e43f16b5cf3 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/MainTest.java @@ -174,7 +174,7 @@ public class MainTest assertThat("jetty.base", baseHome.getBase(), is(homePath.toString())); CommandLineBuilder commandLineBuilder = args.getMainArgs(StartArgs.ALL_PARTS); - String commandLine = commandLineBuilder.toString("\n"); + String commandLine = commandLineBuilder.toString(); String expectedExpansion = String.format("-Xloggc:%s/logs/gc-%s.log", baseHome.getBase(), System.getProperty("java.version") );