From 7721eb102c9dbd62a295b784350cc10efe80ecb1 Mon Sep 17 00:00:00 2001 From: Cheryl King Date: Wed, 6 Mar 2024 20:34:07 -0600 Subject: [PATCH] Handle splitting of containerRunOpts correctly --- .../tools/common/plugins/util/DevUtil.java | 36 ++++++++++++--- .../common/plugins/util/DevUtilTest.java | 45 +++++++++++++++++++ 2 files changed, 74 insertions(+), 7 deletions(-) diff --git a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java index 296afc9e..dd995c4a 100644 --- a/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java +++ b/src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java @@ -1514,7 +1514,34 @@ private boolean alertOnServerError(String line, String errorCode, String gradleM return false; } - private String[] getCommandTokens(String command) { + // Add the passed runOpts to the commandElements as separate options with the option and value specified individually. + // + // Note: If an option's value in runOpts contains spaces and needs quoting/escaping, the getCommandTokens method won't handle it correctly. + // Two ways to approach this: + // 1) parse the opts into separate options ('-xyz abc' or '--xyz abc') and then break the option apart from its value at the first space + // 2) use the returned String[] from getCommandTokens and detect incorrectly broken up option values and splice them back together + // + // Going with option 2. + protected void addContainerRunOpts(String runOpts, List commandElements) { + String[] opts = getCommandTokens(runOpts); + int index = 0; + while (index < opts.length) { + if ((index == 0) || opts[index].startsWith("-")) { + commandElements.add(opts[index]); + } else { + String lastListElement = commandElements.get(commandElements.size()-1); + if (!lastListElement.startsWith("-")) { + // add this to previous element + commandElements.set(commandElements.size()-1, lastListElement+" "+opts[index]); + } else { + commandElements.add(opts[index]); + } + } + index++; + } + } + + protected String[] getCommandTokens(String command) { StringTokenizer stringTokenizer = new StringTokenizer(command); String[] commandTokens = new String[stringTokenizer.countTokens()]; for (int i = 0; stringTokenizer.hasMoreTokens(); i++) { @@ -1678,12 +1705,7 @@ private String[] getContainerCommand() throws IOException, PluginExecutionExcept // Allow the user to add their own options to this command via a system property. if (containerRunOpts != null) { - // If something in containerRunOpts contains spaces and needs quoting/escaping, this getCommandTokens method won't handle it correctly. - // But what was here before would not either. - String[] runOpts = getCommandTokens(containerRunOpts); - for (int i=0; i < runOpts.length; i++) { - commandElements.add(runOpts[i]); - } + addContainerRunOpts(containerRunOpts, commandElements); } // Options must precede this in any order. Image name and command code follows. diff --git a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilTest.java b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilTest.java index 3069b33f..a50e68ed 100644 --- a/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilTest.java +++ b/src/test/java/io/openliberty/tools/common/plugins/util/DevUtilTest.java @@ -34,6 +34,7 @@ import java.nio.file.Files; import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.Map; import javax.xml.stream.FactoryConfigurationError; @@ -47,6 +48,11 @@ public class DevUtilTest extends BaseDevUtilTest { + public static final String RUN_OPTS_SINGLE = "-e MY_OPT=value"; + public static final String RUN_OPTS_SINGLE_WITH_SPACE = "-e MY_OPT=value with space"; + public static final String RUN_OPTS_DOUBLE = "-e MY_OPT=value -e MY_SECOND_OPT=value2"; + public static final String RUN_OPTS_DOUBLE_WITH_SPACE = "-e MY_OPT=value with space -e MY_SECOND_OPT=value 2"; + File serverDirectory; File configDirectory; File srcDir; @@ -132,6 +138,45 @@ public void testCleanupServerEnvBak() throws Exception { assertFalse(serverEnvBak.exists()); } + @Test + public void testContainerRunOpts() throws Exception { + List options = new ArrayList(); + String[] opts = util.getCommandTokens(RUN_OPTS_SINGLE); + assertTrue("RUN_OPTS_SINGLE incorrectly split into "+opts.length+" parameters. Expected 2 parameters.", opts.length == 2); + util.addContainerRunOpts(RUN_OPTS_SINGLE, options); + assertTrue("RUN_OPTS_SINGLE incorrectly split into "+options.size()+" parameters. Expected 2 parameters.", options.size() == 2); + + options.clear(); + opts = util.getCommandTokens(RUN_OPTS_DOUBLE); + assertTrue("RUN_OPTS_DOUBLE incorrectly split into "+opts.length+" parameters. Expected 4 parameters.", opts.length == 4); + util.addContainerRunOpts(RUN_OPTS_DOUBLE, options); + assertTrue("RUN_OPTS_DOUBLE incorrectly split into "+options.size()+" parameters. Expected 4 parameters.", options.size() == 4); + + options.clear(); + opts = util.getCommandTokens(RUN_OPTS_SINGLE_WITH_SPACE); + // note that we really want two see this split into two parameters instead of the four that getCommandTokens returns + assertTrue("RUN_OPTS_SINGLE_WITH_SPACE incorrectly split into "+opts.length+" parameters. Expected 4 parameters.", opts.length == 4); + util.addContainerRunOpts(RUN_OPTS_SINGLE_WITH_SPACE, options); + assertTrue("RUN_OPTS_SINGLE_WITH_SPACE incorrectly split into "+options.size()+" parameters. Expected 2 parameters.", options.size() == 2); + + options.clear(); + opts = util.getCommandTokens(RUN_OPTS_DOUBLE_WITH_SPACE); + // note that we really want two see this split into four parameters instead of the seven that getCommandTokens returns + assertTrue("RUN_OPTS_DOUBLE_WITH_SPACE incorrectly split into "+opts.length+" parameters. Expected 7 parameters.", opts.length == 7); + util.addContainerRunOpts(RUN_OPTS_DOUBLE_WITH_SPACE, options); + assertTrue("RUN_OPTS_DOUBLE_WITH_SPACE incorrectly split into "+options.size()+" parameters. Expected 4 parameters.", options.size() == 4); + + // RUN_OPTS_DOUBLE_WITH_SPACE = "-e MY_OPT=value with space -e MY_SECOND_OPT=value 2"; + List expectedResult = new ArrayList(); + expectedResult.add("-e"); + expectedResult.add("MY_OPT=value with space"); + expectedResult.add("-e"); + expectedResult.add("MY_SECOND_OPT=value 2"); + + assertTrue("List does not contain expected values.", expectedResult.equals(options)); + + } + @Test public void testFindAvailablePort() throws Exception { // prefer a port that is known to be available