From 8c992b9c93dfc505bb619bff1dba662b3d7d7f95 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 6 Feb 2023 15:46:50 -0600 Subject: [PATCH] CommandLineBuilder doesn't escape, and only quotes with single-quote tick when arg has space. Signed-off-by: Joakim Erdfelt --- .../jetty/start/CommandLineBuilder.java | 74 ++++++++----------- .../java/org/eclipse/jetty/start/Main.java | 2 +- .../jetty/start/CommandLineBuilderTest.java | 39 +++++----- 3 files changed, 48 insertions(+), 67 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 b1e5fb5b8e0..ee1e433f7a6 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 @@ -19,17 +19,6 @@ import java.util.List; public class CommandLineBuilder { - // Matrix of 7-bit characters that needs escaping on command line. - private static final boolean[] NEEDS_ESCAPING = new boolean[127]; - - static - { - for (char c : " %$\\{}[]()\"|<>&;`!".toCharArray()) - { - NEEDS_ESCAPING[c] = true; - } - } - public static File findExecutable(File root, String path) { String npath = path.replace('/', File.separatorChar); @@ -70,40 +59,10 @@ public class CommandLineBuilder * * @param arg the argument to quote * @return the quoted and escaped argument - * @deprecated use {@link #escape(String) instead} */ - @Deprecated public static String quote(String arg) { - return escape(arg); - } - - /** - * Escape the raw string to make it suitable for use on a command line. - * - * @param arg the argument to escape - * @return the escaped argument - */ - public static String escape(String arg) - { - StringBuilder buf = new StringBuilder(); - boolean escaped = false; - boolean quoted = false; - for (char c : arg.toCharArray()) - { - if (!quoted && !escaped && NEEDS_ESCAPING[c]) - { - buf.append("\\"); - } - // don't quote text in single quotes - if (!escaped && (c == '\'')) - { - quoted = !quoted; - } - escaped = (c == '\\'); - buf.append(c); - } - return buf.toString(); + return "'" + arg + "'"; } private List args; @@ -130,7 +89,7 @@ public class CommandLineBuilder { if (arg != null) { - args.add(escape(arg)); + args.add(arg); } } @@ -153,11 +112,11 @@ public class CommandLineBuilder { if ((value != null) && (value.length() > 0)) { - args.add(escape(name + "=" + value)); + args.add(name + "=" + value); } else { - args.add(escape(name)); + args.add(name); } } @@ -203,6 +162,31 @@ public class CommandLineBuilder return buf.toString(); } + /** + * A version of {@link #toString()} where every arg is evaluated for potential {@code '} (single-quote tick) wrapping. + * + * @param delim the delimiter between args, use {@code ' '} (space) for shell executable command line. + * @return the toString but each arg that has spaces is surrounded by {@code '} (single-quote tick) + */ + public String toQuotedString(String delim) + { + StringBuilder buf = new StringBuilder(); + + for (String arg : args) + { + if (buf.length() > 0) + buf.append(delim); + boolean needsQuotes = (arg.contains(" ")); + if (needsQuotes) + buf.append("'"); + buf.append(arg); + if (needsQuotes) + buf.append("'"); + } + + return buf.toString(); + } + public void debug() { if (!StartLog.isDebugEnabled()) 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 be8c99fc5b5..25e289ca1cf 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 @@ -440,7 +440,7 @@ public class Main if (args.isDryRun()) { CommandLineBuilder cmd = args.getMainArgs(args.getDryRunParts()); - System.out.println(cmd.toString(StartLog.isDebugEnabled() ? " \\\n" : " ")); + System.out.println(cmd.toQuotedString(StartLog.isDebugEnabled() ? " \\\n" : " ")); } if (args.isStopCommand()) 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 9aab6689d3b..8e8c3de7ae4 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,6 @@ package org.eclipse.jetty.start; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; @@ -27,43 +26,41 @@ public class CommandLineBuilderTest CommandLineBuilder cmd = new CommandLineBuilder("java"); cmd.addEqualsArg("-Djava.io.tmpdir", "/home/java/temp dir/"); cmd.addArg("--version"); - assertThat(cmd.toString(), is("java -Djava.io.tmpdir=/home/java/temp\\ dir/ --version")); + assertThat(cmd.toQuotedString(" "), is("java '-Djava.io.tmpdir=/home/java/temp dir/' --version")); } @Test - public void testEscapedSimple() + public void testSimpleHomeNoSpace() { - assertEscaping("/opt/jetty", "/opt/jetty"); + CommandLineBuilder cmd = new CommandLineBuilder("java"); + cmd.addEqualsArg("-Djetty.home", "/opt/jetty"); + assertThat(cmd.toQuotedString(" "), is("java -Djetty.home=/opt/jetty")); } @Test - public void testEscapedSpaceInPath() + public void testSimpleHomeWithSpace() { - assertEscaping("/opt/jetty 7/home", "/opt/jetty\\ 7/home"); + CommandLineBuilder cmd = new CommandLineBuilder("java"); + cmd.addEqualsArg("-Djetty.home", "/opt/jetty 10/home"); + assertThat(cmd.toQuotedString(" "), is("java '-Djetty.home=/opt/jetty 10/home'")); } - @Test - public void testEscapedSpaceAndQuotesInPath() - { - assertEscaping("/opt/jetty 7 \"special\"/home", "/opt/jetty\\ 7\\ \\\"special\\\"/home"); - } @Test public void testEscapedFormattingString() { - assertEscaping("%{client}a - %u %{dd/MMM/yyyy:HH:mm:ss ZZZ|GMT}t \"%r\" %s %O \"%{Referer}i\" \"%{User-Agent}i\"", - "\\%\\{client\\}a\\ -\\ \\%u\\ \\%\\{dd/MMM/yyyy:HH:mm:ss\\ ZZZ\\|GMT\\}t\\ \\\"\\%r\\\"\\ \\%s\\ \\%O\\ \\\"\\%\\{Referer\\}i\\\"\\ \\\"\\%\\{User-Agent\\}i\\\""); + 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\"'")); } @Test - public void testEscapeQuotationMarks() + public void testEscapeUnicode() { - assertEscaping("-XX:OnOutOfMemoryError='kill -9 %p'", "-XX:OnOutOfMemoryError='kill -9 %p'"); - } - - private void assertEscaping(String raw, String expected) - { - String actual = CommandLineBuilder.escape(raw); - assertThat("Escaped version of [" + raw + "]", actual, is(expected)); + 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=€")); } }