From b56f1bdcdf220f2711a99b2ea443589bf9d3bdf7 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 30 Jan 2012 12:03:03 -0700 Subject: [PATCH] Bug 369349 - space in filename fix broke integration tests + Attempting to fix space and quote issues with new CommandLineBuilder class and tests. --- .../jetty/start/CommandLineBuilder.java | 115 ++++++++++++++++++ .../java/org/eclipse/jetty/start/Main.java | 23 ++-- .../jetty/start/CommandLineBuilderTest.java | 27 ++++ 3 files changed, 154 insertions(+), 11 deletions(-) create mode 100644 jetty-start/src/main/java/org/eclipse/jetty/start/CommandLineBuilder.java create mode 100644 jetty-start/src/test/java/org/eclipse/jetty/start/CommandLineBuilderTest.java 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 new file mode 100644 index 00000000000..ba4e8524cb0 --- /dev/null +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/CommandLineBuilder.java @@ -0,0 +1,115 @@ +package org.eclipse.jetty.start; + +import java.util.ArrayList; +import java.util.List; + +public class CommandLineBuilder { + private List args; + + public CommandLineBuilder(String bin) { + args = new ArrayList(); + } + + /** + * Add a simple argument to the command line. + *

+ * Will quote arguments that have a space in them. + * + * @param arg + * the simple argument to add + */ + public void addArg(String arg) { + args.add(quote(arg)); + } + + /** + * Similar to {@link #addArg(String)} but does no quoting of the name + * parameter, and will quote the value parameter as needed. + *

+ * + * 

+	 *   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) { + if (value != null && value.length() > 0) { + args.add(name + "=" + quote(value)); + } else { + args.add(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 + */ + public void addRawArg(String arg) { + args.add(arg); + } + + public List getArgs() { + return args; + } + + /** + * Perform an optional quoting of the argument, being intelligent with + * spaces and quotes as needed. + * + * @param arg + * @return + */ + public static String quote(String arg) { + boolean needsQuoting = arg.indexOf(' ') >= 0 || arg.indexOf('"') >= 0; + if (!needsQuoting) { + return arg; + } + StringBuilder buf = new StringBuilder(); + buf.append('"'); + boolean escaped = false; + for (char c : arg.toCharArray()) { + if (c == '"') { + if (!escaped) { + buf.append("\\\""); + escaped = false; + continue; + } + } + + if (c == '\\') { + escaped = true; + } + buf.append(c); + } + buf.append('"'); + return buf.toString(); + } + + @Override + public String toString() { + StringBuilder buf = new StringBuilder(); + + boolean delim = false; + for (String arg : args) { + if (delim) { + buf.append(' '); + } + buf.append(arg); + delim = true; + } + + return buf.toString(); + } +} 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 f734d642562..aa1bef143d9 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 @@ -661,20 +661,21 @@ public class Main String buildCommandLine(Classpath classpath, List xmls) throws IOException { - StringBuilder cmd = new StringBuilder(); - cmd.append(findJavaBin()); + CommandLineBuilder cmd = new CommandLineBuilder(findJavaBin()); + for (String x : _jvmArgs) - cmd.append(' ').append(x); - cmd.append(" -Djetty.home=").append(_jettyHome); + { + cmd.addArg(x); + } + cmd.addEqualsArg("-Djetty.home",_jettyHome); for (String p : _sysProps) { - cmd.append(" -D").append(p); String v = System.getProperty(p); - if (v != null && v.length() > 0) - cmd.append('=').append(v); + cmd.addEqualsArg("-D" + p,v); } - cmd.append(" -cp ").append(classpath.toString()); - cmd.append(" ").append(_config.getMainClassname()); + cmd.addArg("-cp"); + cmd.addArg(classpath.toString()); + cmd.addRawArg(_config.getMainClassname()); // Check if we need to pass properties as a file Properties properties = Config.getProperties(); @@ -684,12 +685,12 @@ public class Main if (!_dryRun) prop_file.deleteOnExit(); properties.store(new FileOutputStream(prop_file),"start.jar properties"); - cmd.append(" ").append(prop_file.getAbsolutePath()); + cmd.addArg(prop_file.getAbsolutePath()); } for (String xml : xmls) { - cmd.append(' ').append(xml); + cmd.addArg(xml); } return cmd.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 new file mode 100644 index 00000000000..077d08bbda0 --- /dev/null +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/CommandLineBuilderTest.java @@ -0,0 +1,27 @@ +package org.eclipse.jetty.start; + +import static org.hamcrest.Matchers.*; +import org.junit.Assert; +import org.junit.Test; + +public class CommandLineBuilderTest { + @Test + public void testQuotingSimple() { + assertQuoting("/opt/jetty", "/opt/jetty"); + } + + @Test + public void testQuotingSpaceInPath() { + assertQuoting("/opt/jetty 7/home", "\"/opt/jetty 7/home\""); + } + + @Test + public void testQuotingSpaceAndQuotesInPath() { + assertQuoting("/opt/jetty 7 \"special\"/home", "\"/opt/jetty 7 \\\"special\\\"/home\""); + } + + private void assertQuoting(String raw, String expected) { + String actual = CommandLineBuilder.quote(raw); + Assert.assertThat("Quoted version of [" + raw + "]", actual, is(expected)); + } +}