From 68209deb19055170796c5cc2866b30ab0f51167f Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 12 Jul 2023 09:55:23 +0200 Subject: [PATCH 01/13] Quoting for start arguments backport of #10090 Signed-off-by: gregw --- .../jetty/start/CommandLineBuilder.java | 164 +++++++++++------- .../java/org/eclipse/jetty/start/Main.java | 2 +- .../org/eclipse/jetty/start/StartArgs.java | 62 +++---- .../org/eclipse/jetty/start/usage.txt | 7 +- .../jetty/start/CommandLineBuilderTest.java | 81 +++++++-- .../org/eclipse/jetty/start/MainTest.java | 2 +- 6 files changed, 204 insertions(+), 114 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 7020159ec1fe..dcb8436ab60e 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 @@ -53,31 +53,81 @@ public static String findJavaBin() return "java"; } - /** - * 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. - */ - @Deprecated - public static String quote(String arg) + private final StringBuilder commandLine = new StringBuilder(); + private final List args = new ArrayList<>(); + private final String separator; + + public CommandLineBuilder() { - return "'" + arg + "'"; + this(false); } - private List args; - - public CommandLineBuilder() + public CommandLineBuilder(boolean multiline) { - args = new ArrayList(); + separator = multiline ? (" \\" + System.lineSeparator() + " ") : " "; } - public CommandLineBuilder(String bin) + /** + * Quote a string suitable for use with a command line shell using double quotes. + *

This method applies doubles quoting as described for the unix {@code sh} commands: + * Enclosing characters within double quotes preserves the literal meaning of all characters except + * dollarsign ($), backquote (`), and backslash (\). + * The backslash inside double quotes is historically weird, and serves + * to quote only the following characters: {@code $ ` " \ newline}. + * Otherwise, it remains literal. + * + * @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) { - this(); - args.add(bin); + if (input == null || input.length() == 0) + return input; + + 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 == '_' + ); + } + + 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++); + switch (c) + { + case '"': + case '\\': + case'`': + case'$': + builder.append('\\').appendCodePoint(c); + break; + default: builder.appendCodePoint(c); + } + } + + builder.append('"'); + + return builder.toString(); } /** @@ -92,29 +142,25 @@ public void addArg(String arg) if (arg != null) { args.add(arg); + if (commandLine.length() > 0) + commandLine.append(separator); + 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"
-     *
- * * @param name the name * @param value the value */ - public void addEqualsArg(String name, String value) + public void addArg(String name, String value) { + if (commandLine.length() > 0) + commandLine.append(separator); + commandLine.append(shellQuoteIfNeeded(name)); if ((value != null) && (value.length() > 0)) { args.add(name + "=" + value); + commandLine.append('=').append(shellQuoteIfNeeded(value)); } else { @@ -123,17 +169,31 @@ public void addEqualsArg(String name, String value) } /** - * 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 + * @param option the option + * @param name the name + * @param value the value */ - public void addRawArg(String arg) + public void addArg(String option, String name, String value) { - if (arg != null) + if (commandLine.length() > 0) + commandLine.append(separator); + commandLine.append(option); + if (name == null || name.length() == 0) { - args.add(arg); + args.add(option); + } + else + { + commandLine.append(shellQuoteIfNeeded(name)); + if ((value != null) && (value.length() > 0)) + { + args.add(option + name + "=" + value); + commandLine.append('=').append(shellQuoteIfNeeded(value)); + } + else + { + args.add(option + name); + } } } @@ -144,20 +204,12 @@ public List getArgs() @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 } @@ -169,23 +221,9 @@ public String toString(String delim) * * @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 425805d8e50e..08d991360aac 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 void start(StartArgs args) throws IOException, InterruptedException { 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 13b265ce5ec1..7e1fbe5ffd49 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; @@ -714,13 +715,13 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException 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.addArg("-D", "java.io.tmpdir", System.getProperty("java.io.tmpdir")); + cmd.addArg("-D", "jetty.home", baseHome.getHome()); + cmd.addArg("-D", "jetty.base", baseHome.getBase()); for (String x : getJvmArgSources().keySet()) { @@ -731,11 +732,11 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException String value = assign.length == 1 ? "" : assign[1]; Prop p = processSystemProperty(key, value, null); - cmd.addRawArg("-D" + p.key + "=" + getProperties().expand(p.value)); + cmd.addArg("-D", p.key, properties.expand(p.value)); } else { - cmd.addRawArg(getProperties().expand(x)); + cmd.addArg(getProperties().expand(x)); } } @@ -743,7 +744,7 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException for (String propKey : systemPropertySource.keySet()) { String value = System.getProperty(propKey); - cmd.addEqualsArg("-D" + propKey, value); + cmd.addArg("-D", propKey, value); } } @@ -756,60 +757,56 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException List files = dirsAndFiles.get(false); if (files != null && !files.isEmpty()) { - cmd.addRawArg("--module-path"); + cmd.addArg("--module-path", null, null); 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.addArg("--class-path", null, null); 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.addArg("--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.addArg("--patch-module", 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.addArg("--add-opens", 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.addArg("--add-exports", 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.addArg("--add-reads", entry.getKey(), String.join(",", entry.getValue())); } } else { - cmd.addRawArg("--class-path"); - cmd.addRawArg(classpath.toString()); + cmd.addArg("--class-path", null, null); + cmd.addArg(classpath.toString()); } } if (parts.contains("main")) { if (isJPMS()) - cmd.addRawArg("--module"); - cmd.addRawArg(getMainClassname()); + cmd.addArg("--module", null, null); + cmd.addArg(getMainClassname()); } // pass properties as args or as a file @@ -819,7 +816,8 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException { for (Prop p : properties) { - cmd.addRawArg(CommandLineBuilder.quote(p.key) + "=" + CommandLineBuilder.quote(properties.expand(p.value))); + if (!p.key.startsWith("java.")) + cmd.addArg(p.key, properties.expand(p.value)); } } else if (properties.size() > 0) @@ -837,17 +835,17 @@ else if (properties.size() > 0) { 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 +1223,12 @@ public void parse(final String rawarg, String source) 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 19a865b7ccdc..a13ed1a9cc6d 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,17 +60,20 @@ 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=(,)* - Prints specific parts of the command line. The parts are: + Prints specific parts of the command line in a format usable by + a 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 o "main" - the main class to run o "args" - the arguments passed to the main class + o "multipart" - a fake part that causes the command to be + generated over multiple lines Configure Commands: ------------------- 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 06753229e7e9..975e60fe3b63 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,83 @@ 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("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("'single'", "\"'single'\"") + ); + } + + @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=\"€\"")); } -} +} \ No newline at end of file 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 94a3ddafa5b8..e43f16b5cf33 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 void testJvmArgExpansion() throws Exception 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") ); From a85ccf5478917feedb6bee43a3dfe3833714e100 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 12 Jul 2023 10:44:13 +0200 Subject: [PATCH 02/13] Quoting for start arguments Do not quote comma Signed-off-by: gregw --- .../main/java/org/eclipse/jetty/start/CommandLineBuilder.java | 1 + 1 file changed, 1 insertion(+) 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 dcb8436ab60e..643101b95f96 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 @@ -98,6 +98,7 @@ public static String shellQuoteIfNeeded(String input) c == '/' || c == ':' || c == '.' || + c == ',' || c == '-' || c == '_' ); From 26af0c86794288844a1e84e46e3a22e7a6095874 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 12 Jul 2023 12:31:33 +0200 Subject: [PATCH 03/13] Quoting for start arguments Updates from review Signed-off-by: gregw --- .../jetty/start/CommandLineBuilder.java | 54 +++++++++++-------- .../org/eclipse/jetty/start/StartArgs.java | 2 +- .../org/eclipse/jetty/start/usage.txt | 4 +- .../jetty/start/CommandLineBuilderTest.java | 6 ++- 4 files changed, 39 insertions(+), 27 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 643101b95f96..bbb6f13b19ca 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 @@ import java.io.File; import java.util.ArrayList; import java.util.List; +import java.util.Objects; public class CommandLineBuilder { @@ -81,8 +82,10 @@ public CommandLineBuilder(boolean multiline) */ public static String shellQuoteIfNeeded(String input) { - if (input == null || input.length() == 0) - return input; + if (input == null) + return null; + if (input.length() == 0) + return "\"\""; int i = 0; boolean needsQuoting = false; @@ -118,11 +121,11 @@ public static String shellQuoteIfNeeded(String input) { case '"': case '\\': - case'`': - case'$': - builder.append('\\').appendCodePoint(c); + case '`': + case '$': + builder.append('\\').append(c); break; - default: builder.appendCodePoint(c); + default: builder.append(c); } } @@ -132,9 +135,7 @@ public static String shellQuoteIfNeeded(String input) } /** - * Add a simple argument to the command line. - *

- * Will quote arguments that have a space in them. + * Add a simple argument to the command line, quoted if necessary. * * @param arg the simple argument to add */ @@ -142,59 +143,66 @@ public void addArg(String arg) { if (arg != null) { - args.add(arg); if (commandLine.length() > 0) commandLine.append(separator); + args.add(arg); commandLine.append(shellQuoteIfNeeded(arg)); } } /** + * 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 addArg(String name, String value) { + Objects.requireNonNull(name); + if (commandLine.length() > 0) commandLine.append(separator); - commandLine.append(shellQuoteIfNeeded(name)); + if ((value != null) && (value.length() > 0)) { args.add(name + "=" + value); - commandLine.append('=').append(shellQuoteIfNeeded(value)); + commandLine.append(shellQuoteIfNeeded(name)).append('=').append(shellQuoteIfNeeded(value)); } else { args.add(name); + commandLine.append(shellQuoteIfNeeded(name)); } } /** + * Add a "-Oname=value" style argument to the command line with + * name and value quoted if necessary. * @param option the option * @param name the name * @param value the value */ public void addArg(String option, String name, String value) { + Objects.requireNonNull(option); + if (commandLine.length() > 0) commandLine.append(separator); - commandLine.append(option); + if (name == null || name.length() == 0) { + 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 { - commandLine.append(shellQuoteIfNeeded(name)); - if ((value != null) && (value.length() > 0)) - { - args.add(option + name + "=" + value); - commandLine.append('=').append(shellQuoteIfNeeded(value)); - } - else - { - args.add(option + name); - } + args.add(option + name); + commandLine.append(option).append(shellQuoteIfNeeded(name)); } } 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 7e1fbe5ffd49..8c0efc9f9c94 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 @@ -707,7 +707,7 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException if (parts.isEmpty()) parts = ALL_PARTS; - CommandLineBuilder cmd = new CommandLineBuilder(); + CommandLineBuilder cmd = new CommandLineBuilder(multiLine); // Special Stop/Shutdown properties ensureSystemPropertySet("STOP.PORT"); 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 a13ed1a9cc6d..dbbc074f317f 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 @@ -66,13 +66,13 @@ Report Commands: --dry-run=(,)* Prints specific parts of the command line in a format usable by - a the unix sh command. The parts are: + 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 o "main" - the main class to run o "args" - the arguments passed to the main class - o "multipart" - a fake part that causes the command to be + o "multiline" - a fake part that causes the command to be generated over multiple lines Configure Commands: 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 975e60fe3b63..fa75241d6c18 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 @@ -77,9 +77,13 @@ public static Stream shellQuoting() { return Stream.of( Arguments.of(null, null), - Arguments.of("", ""), + 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("H-llo_world", "H-llo_world"), Arguments.of("H:llo/world", "H:llo/world"), Arguments.of("Hello World", "\"Hello World\""), From d65ca77b05a93e10a6fd409c6b2ba754c11dc892 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 12 Jul 2023 16:47:32 +0200 Subject: [PATCH 04/13] Quoting for start arguments Updates from review Signed-off-by: gregw --- .../main/java/org/eclipse/jetty/start/CommandLineBuilder.java | 3 ++- .../src/main/resources/org/eclipse/jetty/start/usage.txt | 4 ++-- 2 files changed, 4 insertions(+), 3 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 bbb6f13b19ca..ad9d5e7f455b 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 @@ -70,7 +70,7 @@ public CommandLineBuilder(boolean multiline) /** * Quote a string suitable for use with a command line shell using double quotes. - *

This method applies doubles quoting as described for the unix {@code sh} commands: + *

This method applies doubles quotes suitable for a POSIX compliant shell: * Enclosing characters within double quotes preserves the literal meaning of all characters except * dollarsign ($), backquote (`), and backslash (\). * The backslash inside double quotes is historically weird, and serves @@ -123,6 +123,7 @@ public static String shellQuoteIfNeeded(String input) case '\\': case '`': case '$': + case '\n': builder.append('\\').append(c); break; default: builder.append(c); 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 dbbc074f317f..17a2167a960f 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,13 +60,13 @@ Report Commands: --dry-run Prints the command line that start.jar generates, - in a format usable by the unix sh command, 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 in a format usable by - the unix sh command. The parts are: + 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 From df91bfd31915ed82637827694c78504d9a19a993 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 13 Jul 2023 12:00:45 +0200 Subject: [PATCH 05/13] Quoting for start arguments Revert to single quotes for the sake of xargs in jetty.sh Signed-off-by: gregw --- .../jetty/start/CommandLineBuilder.java | 40 +++++++++---------- .../org/eclipse/jetty/start/StartArgs.java | 2 +- .../jetty/start/CommandLineBuilderTest.java | 26 ++++++------ 3 files changed, 34 insertions(+), 34 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 ad9d5e7f455b..2e3acb348f28 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 @@ -69,23 +69,21 @@ public CommandLineBuilder(boolean multiline) } /** - * Quote a string suitable for use with a command line shell using double quotes. - *

This method applies doubles quotes suitable for a POSIX compliant shell: - * Enclosing characters within double quotes preserves the literal meaning of all characters except - * dollarsign ($), backquote (`), and backslash (\). - * The backslash inside double quotes is historically weird, and serves - * to quote only the following characters: {@code $ ` " \ newline}. - * Otherwise, it remains literal. + * 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 process differently by some shells and the xarg + // command used by jetty.xh + if (input == null) return null; if (input.length() == 0) - return "\"\""; + return "''"; int i = 0; boolean needsQuoting = false; @@ -111,26 +109,28 @@ public static String shellQuoteIfNeeded(String input) return input; StringBuilder builder = new StringBuilder(input.length() * 2); - builder.append('"'); + builder.append('\''); builder.append(input, 0, --i); while (i < input.length()) { char c = input.charAt(i++); - switch (c) + if (c == '\'') { - case '"': - case '\\': - case '`': - case '$': - case '\n': - builder.append('\\').append(c); - break; - default: builder.append(c); + // There is no escape for a literal single quote, so we must leave the quotes + // and then escape the single quote. If we are at the start or end of the string + // we can be less ugly and avoid an empty '' + if (i == 1) + builder.insert(0, '\\').append('\''); + else if (i == input.length()) + builder.append("'\\"); + else + builder.append("'\\''"); } + else + builder.append(c); } - - builder.append('"'); + builder.append('\''); return builder.toString(); } 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 8c0efc9f9c94..124c8ffcda9f 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 @@ -816,7 +816,7 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException { for (Prop p : properties) { - if (!p.key.startsWith("java.")) + if (!p.key.startsWith("java.version.")) cmd.addArg(p.key, properties.expand(p.value)); } } 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 fa75241d6c18..bd7fafa1d815 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 @@ -32,7 +32,7 @@ public void testSimpleCommandline() cmd.addArg("java"); cmd.addArg("-Djava.io.tmpdir", "/home/java/temp dir/"); cmd.addArg("--version"); - assertThat(cmd.toCommandLine(), is("java -Djava.io.tmpdir=\"/home/java/temp dir/\" --version")); + assertThat(cmd.toCommandLine(), is("java -Djava.io.tmpdir='/home/java/temp dir/' --version")); } @Test @@ -50,7 +50,7 @@ public void testSimpleHomeWithSpace() 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\"")); + assertThat(cmd.toCommandLine(), is("java -Djetty.home='/opt/jetty 10/home'")); } @Test @@ -60,7 +60,7 @@ public void testEscapedFormattingString() 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\\\"\"")); + 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 @@ -70,25 +70,25 @@ public void testEscapeUnicode() cmd.addArg("java"); cmd.addArg("-Djetty.home", "/opt/jetty"); cmd.addArg("monetary.symbol", "€"); - assertThat(cmd.toCommandLine(), is("java -Djetty.home=/opt/jetty 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("", "''"), 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("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("'single'", "\"'single'\"") + Arguments.of("Hello World", "'Hello World'"), + Arguments.of("foo\\bar", "'foo\\bar'") ); } @@ -109,6 +109,6 @@ public void testMultiLine() assertThat(cmd.toCommandLine(), is("java \\" + System.lineSeparator() + " -Djetty.home=/opt/jetty \\" + System.lineSeparator() + - " monetary.symbol=\"€\"")); + " monetary.symbol='€'")); } } \ No newline at end of file From f4a2deaf2fdae16ec55ea58650097c64a4679ed9 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 13 Jul 2023 12:08:48 +0200 Subject: [PATCH 06/13] Quoting for start arguments restored deprecated APIs Signed-off-by: gregw --- .../jetty/start/CommandLineBuilder.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) 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 2e3acb348f28..d301e4ec050c 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 @@ -54,6 +54,17 @@ public static String findJavaBin() return "java"; } + /** + * @param arg string + * @return Quoted string + * @deprecated use {@link #shellQuoteIfNeeded(String)} + */ + @Deprecated + public static String quote(String arg) + { + return "'" + arg + "'"; + } + private final StringBuilder commandLine = new StringBuilder(); private final List args = new ArrayList<>(); private final String separator; @@ -63,6 +74,13 @@ public CommandLineBuilder() this(false); } + @Deprecated + public CommandLineBuilder(String bin) + { + this(); + args.add(bin); + } + public CommandLineBuilder(boolean multiline) { separator = multiline ? (" \\" + System.lineSeparator() + " ") : " "; @@ -151,6 +169,15 @@ public void addArg(String arg) } } + /** + * @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. @@ -176,6 +203,15 @@ public void addArg(String name, String value) } } + /** + * @deprecated use {@link #addArg(String)} + */ + @Deprecated + public void addRawArg(String arg) + { + addArg(arg); + } + /** * Add a "-Oname=value" style argument to the command line with * name and value quoted if necessary. @@ -226,6 +262,15 @@ public String toString() 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. * From ff67adede0d843b598f895109be82ad2c5d5e4a6 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 13 Jul 2023 14:03:11 +0200 Subject: [PATCH 07/13] Quoting for start arguments fixed JPMS args Signed-off-by: gregw --- .../main/java/org/eclipse/jetty/start/StartArgs.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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 124c8ffcda9f..6937cf67cfea 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 @@ -780,19 +780,23 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException } for (Map.Entry> entry : jmodPatch.entrySet()) { - cmd.addArg("--patch-module", entry.getKey(), String.join(File.pathSeparator, entry.getValue())); + cmd.addArg("--patch-module", null, null); + cmd.addArg(entry.getKey(), String.join(File.pathSeparator, entry.getValue())); } for (Map.Entry> entry : jmodOpens.entrySet()) { - cmd.addArg("--add-opens", entry.getKey(), String.join(",", entry.getValue())); + cmd.addArg("--add-opens", null, null); + cmd.addArg(entry.getKey(), String.join(",", entry.getValue())); } for (Map.Entry> entry : jmodExports.entrySet()) { - cmd.addArg("--add-exports", entry.getKey(), String.join(",", entry.getValue())); + cmd.addArg("--add-exports", null, null); + cmd.addArg(entry.getKey(), String.join(",", entry.getValue())); } for (Map.Entry> entry : jmodReads.entrySet()) { - cmd.addArg("--add-reads", entry.getKey(), String.join(",", entry.getValue())); + cmd.addArg("--add-reads", null, null); + cmd.addArg(entry.getKey(), String.join(",", entry.getValue())); } } else From 44c1fe2ce668528db623c9fbee02e3be22396d7a Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 13 Jul 2023 14:16:49 +0200 Subject: [PATCH 08/13] Quoting for start arguments options cleaned up Signed-off-by: gregw --- .../jetty/start/CommandLineBuilder.java | 17 ++++++++++-- .../org/eclipse/jetty/start/StartArgs.java | 26 +++++++++---------- 2 files changed, 28 insertions(+), 15 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 d301e4ec050c..8f088bb1a3b3 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 @@ -213,13 +213,26 @@ public void addRawArg(String arg) } /** - * Add a "-Oname=value" style argument to the command line with + * Add a "-Oname" style option to the command line with no quoting. + * @param option the option + */ + public void addOption(String option) + { + Objects.requireNonNull(option); + if (commandLine.length() > 0) + commandLine.append(separator); + commandLine.append(option); + args.add(option); + } + + /** + * Add a "-Oname=value" style option to the command line with * name and value quoted if necessary. * @param option the option * @param name the name * @param value the value */ - public void addArg(String option, String name, String value) + public void addOption(String option, String name, String value) { Objects.requireNonNull(option); 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 6937cf67cfea..a9221ef2a897 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 @@ -719,9 +719,9 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException if (parts.contains("opts")) { - cmd.addArg("-D", "java.io.tmpdir", System.getProperty("java.io.tmpdir")); - cmd.addArg("-D", "jetty.home", baseHome.getHome()); - cmd.addArg("-D", "jetty.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()) { @@ -732,7 +732,7 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException String value = assign.length == 1 ? "" : assign[1]; Prop p = processSystemProperty(key, value, null); - cmd.addArg("-D", p.key, properties.expand(p.value)); + cmd.addOption("-D", p.key, properties.expand(p.value)); } else { @@ -744,7 +744,7 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException for (String propKey : systemPropertySource.keySet()) { String value = System.getProperty(propKey); - cmd.addArg("-D", propKey, value); + cmd.addOption("-D", propKey, value); } } @@ -757,7 +757,7 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException List files = dirsAndFiles.get(false); if (files != null && !files.isEmpty()) { - cmd.addArg("--module-path", null, null); + cmd.addOption("--module-path"); String modules = files.stream() .map(File::getAbsolutePath) .collect(Collectors.joining(File.pathSeparator)); @@ -766,7 +766,7 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException List dirs = dirsAndFiles.get(true); if (dirs != null && !dirs.isEmpty()) { - cmd.addArg("--class-path", null, null); + cmd.addOption("--class-path"); String directories = dirs.stream() .map(File::getAbsolutePath) .collect(Collectors.joining(File.pathSeparator)); @@ -780,28 +780,28 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException } for (Map.Entry> entry : jmodPatch.entrySet()) { - cmd.addArg("--patch-module", null, null); + cmd.addOption("--patch-module"); cmd.addArg(entry.getKey(), String.join(File.pathSeparator, entry.getValue())); } for (Map.Entry> entry : jmodOpens.entrySet()) { - cmd.addArg("--add-opens", null, null); + cmd.addOption("--add-opens"); cmd.addArg(entry.getKey(), String.join(",", entry.getValue())); } for (Map.Entry> entry : jmodExports.entrySet()) { - cmd.addArg("--add-exports", null, null); + cmd.addOption("--add-exports"); cmd.addArg(entry.getKey(), String.join(",", entry.getValue())); } for (Map.Entry> entry : jmodReads.entrySet()) { - cmd.addArg("--add-reads", null, null); + cmd.addOption("--add-reads"); cmd.addArg(entry.getKey(), String.join(",", entry.getValue())); } } else { - cmd.addArg("--class-path", null, null); + cmd.addOption("--class-path"); cmd.addArg(classpath.toString()); } } @@ -809,7 +809,7 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException if (parts.contains("main")) { if (isJPMS()) - cmd.addArg("--module", null, null); + cmd.addOption("--module"); cmd.addArg(getMainClassname()); } From 845fbb1fc4aeaf3e0ed63ce3be3b141bce4bf4ba Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 13 Jul 2023 17:09:51 +0200 Subject: [PATCH 09/13] Apply suggestions from code review Co-authored-by: Simone Bordet --- .../eclipse/jetty/start/CommandLineBuilder.java | 14 +++++--------- .../java/org/eclipse/jetty/start/StartArgs.java | 2 +- .../jetty/start/CommandLineBuilderTest.java | 3 ++- 3 files changed, 8 insertions(+), 11 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 8f088bb1a3b3..00891bbad14c 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 @@ -95,8 +95,8 @@ public CommandLineBuilder(boolean multiline) */ public static String shellQuoteIfNeeded(String input) { - // Single quotes are used because double quotes are process differently by some shells and the xarg - // command used by jetty.xh + // Single quotes are used because double quotes are processed differently by some shells and the xarg + // command used by jetty.sh if (input == null) return null; @@ -136,8 +136,8 @@ public static String shellQuoteIfNeeded(String input) if (c == '\'') { // There is no escape for a literal single quote, so we must leave the quotes - // and then escape the single quote. If we are at the start or end of the string - // we can be less ugly and avoid an empty '' + // 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()) @@ -218,11 +218,7 @@ public void addRawArg(String arg) */ public void addOption(String option) { - Objects.requireNonNull(option); - if (commandLine.length() > 0) - commandLine.append(separator); - commandLine.append(option); - args.add(option); + addOption(option, null, null); } /** 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 a9221ef2a897..b1554fa0237f 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 @@ -775,7 +775,7 @@ public CommandLineBuilder getMainArgs(Set parts) throws IOException if (!jmodAdds.isEmpty()) { - cmd.addArg("--add-modules"); + cmd.addOption("--add-modules"); cmd.addArg(String.join(",", jmodAdds)); } for (Map.Entry> entry : jmodPatch.entrySet()) 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 bd7fafa1d815..abfa6b25e965 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 @@ -88,7 +88,8 @@ public static Stream shellQuoting() 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("foo'bar", "'foo'bar'") ); } From 61b51bad8b3b70202e23aea067b1947d508ca1a1 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 13 Jul 2023 17:14:32 +0200 Subject: [PATCH 10/13] Quoting for start arguments updates from review Signed-off-by: gregw --- .../java/org/eclipse/jetty/start/CommandLineBuilder.java | 8 ++++---- .../org/eclipse/jetty/start/CommandLineBuilderTest.java | 4 ++-- 2 files changed, 6 insertions(+), 6 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 00891bbad14c..c601b5502ad9 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 @@ -127,7 +127,7 @@ public static String shellQuoteIfNeeded(String input) return input; StringBuilder builder = new StringBuilder(input.length() * 2); - builder.append('\''); + builder.append("'"); builder.append(input, 0, --i); while (i < input.length()) @@ -136,10 +136,10 @@ public static String shellQuoteIfNeeded(String input) 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 + // 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('\''); + builder.insert(0, "\\").append("'"); else if (i == input.length()) builder.append("'\\"); else @@ -148,7 +148,7 @@ else if (i == input.length()) else builder.append(c); } - builder.append('\''); + builder.append("'"); return builder.toString(); } 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 abfa6b25e965..e4050d75ebec 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 @@ -89,7 +89,7 @@ public static Stream shellQuoting() 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("foo'bar", "'foo'\\''bar'") ); } @@ -112,4 +112,4 @@ public void testMultiLine() " -Djetty.home=/opt/jetty \\" + System.lineSeparator() + " monetary.symbol='€'")); } -} \ No newline at end of file +} From ab25cd96874d0cbb341c0fb3513f38921e6908ec Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 13 Jul 2023 17:40:19 +0200 Subject: [PATCH 11/13] Apply suggestions from code review Co-authored-by: Simone Bordet --- .../java/org/eclipse/jetty/start/CommandLineBuilder.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 c601b5502ad9..0067b5bec6b3 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 @@ -213,7 +213,7 @@ public void addRawArg(String arg) } /** - * Add a "-Oname" style option to the command line with no quoting. + * Adds a "-O" style option to the command line with no quoting, for example `--help`. * @param option the option */ public void addOption(String option) @@ -222,8 +222,8 @@ public void addOption(String option) } /** - * Add a "-Oname=value" style option to the command line with - * name and value quoted if necessary. + * Adds a "-Oname=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 From 6871906d8d99c00062e2cb68535ddc1cd61a99fc Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 13 Jul 2023 22:52:39 +0200 Subject: [PATCH 12/13] Quoting for start arguments updates from review Signed-off-by: gregw --- .../java/org/eclipse/jetty/start/CommandLineBuilder.java | 5 +++-- 1 file changed, 3 insertions(+), 2 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 0067b5bec6b3..ece449f0ef21 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 @@ -118,6 +118,7 @@ public static String shellQuoteIfNeeded(String input) c == ':' || c == '.' || c == ',' || + c == '+' || c == '-' || c == '_' ); @@ -213,7 +214,7 @@ public void addRawArg(String arg) } /** - * Adds a "-O" style option to the command line with no quoting, for example `--help`. + * Adds a "-OPTION" style option to the command line with no quoting, for example `--help`. * @param option the option */ public void addOption(String option) @@ -222,7 +223,7 @@ public void addOption(String option) } /** - * Adds a "-Oname=value" style option to the command line with + * 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 From 4ecf5215f7a938b645d74df4dcb550d633be274e Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 14 Jul 2023 17:00:28 +0200 Subject: [PATCH 13/13] * Made "dry-run=multiline" hidden. * A couple more test cases. Signed-off-by: Simone Bordet --- .../java/org/eclipse/jetty/start/CommandLineBuilder.java | 4 ++-- .../src/main/resources/org/eclipse/jetty/start/usage.txt | 6 ++---- .../org/eclipse/jetty/start/CommandLineBuilderTest.java | 4 +++- 3 files changed, 7 insertions(+), 7 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 ece449f0ef21..ca1f9f900b7c 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 @@ -95,8 +95,8 @@ public CommandLineBuilder(boolean multiline) */ public static String shellQuoteIfNeeded(String input) { - // Single quotes are used because double quotes are processed differently by some shells and the xarg - // command used by jetty.sh + // Single quotes are used because double quotes + // are evaluated differently by some shells. if (input == null) return null; 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 17a2167a960f..5813c199fddd 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,20 +60,18 @@ Report Commands: --dry-run Prints the command line that start.jar generates, - in a format usable by a posix compliant shell, 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 in a format usable by - a posix compliant shell. The parts are: + 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 o "main" - the main class to run o "args" - the arguments passed to the main class - o "multiline" - a fake part that causes the command to be - generated over multiple lines Configure Commands: ------------------- 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 e4050d75ebec..6f938f7ae00a 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 @@ -89,7 +89,9 @@ public static Stream shellQuoting() 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("foo'bar", "'foo'\\''bar'"), + Arguments.of("some 'internal' quoting", "'some '\\''internal'\\'' quoting'"), + Arguments.of("monetary.symbol=€", "'monetary.symbol=€'") ); }