Merge pull request #16471 from s1monw/no_sys_props_leaked

Apply system properties after all arguemnts are parsed in BootstrapCLIParser
This commit is contained in:
Simon Willnauer 2016-02-05 14:23:00 +01:00
commit 9b2f40f627
2 changed files with 20 additions and 3 deletions

View File

@ -32,10 +32,12 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment; import org.elasticsearch.env.Environment;
import org.elasticsearch.monitor.jvm.JvmInfo; import org.elasticsearch.monitor.jvm.JvmInfo;
import java.util.HashMap;
import java.util.Iterator; import java.util.Iterator;
import java.util.Locale; import java.util.Locale;
import java.util.Map; import java.util.Map;
import java.util.Properties; import java.util.Properties;
import java.util.Set;
import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd; import static org.elasticsearch.common.cli.CliToolConfig.Builder.cmd;
import static org.elasticsearch.common.cli.CliToolConfig.Builder.optionBuilder; import static org.elasticsearch.common.cli.CliToolConfig.Builder.optionBuilder;
@ -131,6 +133,7 @@ final class BootstrapCLIParser extends CliTool {
// hacky way to extract all the fancy extra args, there is no CLI tool helper for this // hacky way to extract all the fancy extra args, there is no CLI tool helper for this
Iterator<String> iterator = cli.getArgList().iterator(); Iterator<String> iterator = cli.getArgList().iterator();
final Map<String, String> properties = new HashMap<>();
while (iterator.hasNext()) { while (iterator.hasNext()) {
String arg = iterator.next(); String arg = iterator.next();
if (!arg.startsWith("--")) { if (!arg.startsWith("--")) {
@ -148,20 +151,22 @@ final class BootstrapCLIParser extends CliTool {
String[] splitArg = arg.split("=", 2); String[] splitArg = arg.split("=", 2);
String key = splitArg[0]; String key = splitArg[0];
String value = splitArg[1]; String value = splitArg[1];
System.setProperty("es." + key, value); properties.put("es." + key, value);
} else { } else {
if (iterator.hasNext()) { if (iterator.hasNext()) {
String value = iterator.next(); String value = iterator.next();
if (value.startsWith("--")) { if (value.startsWith("--")) {
throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "] needs value"); throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "] needs value");
} }
System.setProperty("es." + arg, value); properties.put("es." + arg, value);
} else { } else {
throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "] needs value"); throw new UserError(ExitStatus.USAGE, "Parameter [" + arg + "] needs value");
} }
} }
} }
for (Map.Entry<String, String> entry : properties.entrySet()) {
System.setProperty(entry.getKey(), entry.getValue());
}
return new Start(terminal); return new Start(terminal);
} }

View File

@ -29,11 +29,15 @@ import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.monitor.jvm.JvmInfo; import org.elasticsearch.monitor.jvm.JvmInfo;
import org.hamcrest.Matcher; import org.hamcrest.Matcher;
import org.junit.After; import org.junit.After;
import org.junit.Before;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Locale; import java.util.Locale;
import java.util.Map;
import java.util.Properties;
import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK; import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK;
import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK_AND_EXIT; import static org.elasticsearch.common.cli.CliTool.ExitStatus.OK_AND_EXIT;
@ -48,6 +52,12 @@ public class BootstrapCliParserTests extends CliToolTestCase {
private CaptureOutputTerminal terminal = new CaptureOutputTerminal(); private CaptureOutputTerminal terminal = new CaptureOutputTerminal();
private List<String> propertiesToClear = new ArrayList<>(); private List<String> propertiesToClear = new ArrayList<>();
private Map<Object, Object> properties;
@Before
public void before() {
this.properties = new HashMap<>(System.getProperties());
}
@After @After
public void clearProperties() { public void clearProperties() {
@ -55,6 +65,7 @@ public class BootstrapCliParserTests extends CliToolTestCase {
System.clearProperty(property); System.clearProperty(property);
} }
propertiesToClear.clear(); propertiesToClear.clear();
assertEquals("properties leaked", properties, new HashMap<>(System.getProperties()));
} }
public void testThatVersionIsReturned() throws Exception { public void testThatVersionIsReturned() throws Exception {
@ -235,6 +246,7 @@ public class BootstrapCliParserTests extends CliToolTestCase {
parser.parse("start", new String[]{"--foo=bar", "-Dbaz=qux"}); parser.parse("start", new String[]{"--foo=bar", "-Dbaz=qux"});
}); });
assertThat(e.getMessage(), containsString("must be before any parameters starting with --")); assertThat(e.getMessage(), containsString("must be before any parameters starting with --"));
assertNull(System.getProperty("es.foo"));
} }
private void registerProperties(String ... systemProperties) { private void registerProperties(String ... systemProperties) {