diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java index 4dd7dfec2..86034e46f 100644 --- a/src/main/java/org/apache/commons/cli/DefaultParser.java +++ b/src/main/java/org/apache/commons/cli/DefaultParser.java @@ -505,15 +505,16 @@ private void handleProperties(final Properties properties) throws ParseException if (!cmd.hasOption(option) && !selected) { // get the value from the properties final String value = properties.getProperty(option); - if (opt.hasArg()) { - if (opt.isValuesEmpty()) { - opt.processValue(stripLeadingAndTrailingQuotesDefaultOff(value)); - } - } else if (!("yes".equalsIgnoreCase(value) || "true".equalsIgnoreCase(value) || "1".equalsIgnoreCase(value))) { + if (!opt.hasArg() && !("yes".equalsIgnoreCase(value) || "true".equalsIgnoreCase(value) || "1".equalsIgnoreCase(value))) { // if the value is not yes, true or 1 then don't add the option to the CommandLine continue; } + // handleOption() adds a copy of the option to the CommandLine; apply the value to that copy, not to the + // option registered in options, otherwise the value survives this parse() call and is reused by the next one handleOption(opt); + if (currentOption != null && currentOption.isValuesEmpty()) { + currentOption.processValue(stripLeadingAndTrailingQuotesDefaultOff(value)); + } currentOption = null; } } diff --git a/src/test/java/org/apache/commons/cli/AbstractParserTestCase.java b/src/test/java/org/apache/commons/cli/AbstractParserTestCase.java index 58657821a..57371a448 100644 --- a/src/test/java/org/apache/commons/cli/AbstractParserTestCase.java +++ b/src/test/java/org/apache/commons/cli/AbstractParserTestCase.java @@ -667,6 +667,23 @@ void testReuseOptionsTwice() throws Exception { assertThrows(MissingOptionException.class, () -> parser.parse(options, new String[0])); } + @Test + void testReuseOptionsWithProperties() throws Exception { + final Options options = new Options(); + options.addOption(OptionBuilder.hasArg().create('a')); + // first parsing, the value comes from the properties + final Properties properties = new Properties(); + properties.setProperty("a", "one"); + assertEquals("one", parse(parser, options, null, properties).getOptionValue("a")); + // second parsing, with the same Options instance and the value on the command line + final CommandLine cmd = parser.parse(options, new String[] { "-a", "two" }); + assertEquals("two", cmd.getOptionValue("a")); + assertEquals(0, cmd.getArgList().size()); + // third parsing, with the same Options instance and another default value + properties.setProperty("a", "three"); + assertEquals("three", parse(parser, options, null, properties).getOptionValue("a")); + } + @Test void testShortOptionConcatenatedQuoteHandling() throws Exception { final CommandLine cl = parser.parse(options, new String[] { "-b\"quoted string\"" }); diff --git a/src/test/java/org/apache/commons/cli/DefaultParserTest.java b/src/test/java/org/apache/commons/cli/DefaultParserTest.java index c162323f0..d1734dc08 100644 --- a/src/test/java/org/apache/commons/cli/DefaultParserTest.java +++ b/src/test/java/org/apache/commons/cli/DefaultParserTest.java @@ -23,6 +23,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.HashSet; +import java.util.Properties; import java.util.Set; import java.util.stream.Stream; @@ -400,6 +401,17 @@ void testParseSkipNonHappyPath() throws ParseException { assertTrue(e.getMessage().contains("-d")); } + @Test + void testPropertyDoesNotOverridePresetValue() throws ParseException { + final Option a = Option.builder("a").hasArg().get(); + a.getValuesList().add("preset"); + final Options options = new Options().addOption(a); + final Properties properties = new Properties(); + properties.setProperty("a", "fromProperties"); + assertEquals("preset", new DefaultParser().parse(options, null, properties).getOptionValue("a")); + assertEquals("[preset]", a.getValuesList().toString()); + } + @Override @Test @Disabled("Test case handled in the parameterized tests as \"DEFAULT behavior\"")