Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle splitting of containerRunOpts correctly #445

Merged
merged 2 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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++) {
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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_EXTRA_SPACES = " -e MY_OPT=value with space -e MY_SECOND_OPT=value 2";

File serverDirectory;
File configDirectory;
File srcDir;
Expand Down Expand Up @@ -132,6 +138,45 @@ public void testCleanupServerEnvBak() throws Exception {
assertFalse(serverEnvBak.exists());
}

@Test
public void testContainerRunOpts() throws Exception {
List<String> options = new ArrayList<String>();
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 to 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_EXTRA_SPACES);
// note that we really want to 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_EXTRA_SPACES, 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<String> expectedResult = new ArrayList<String>();
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
Expand Down
Loading