diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java index 9f9285d78..6bd5bb391 100644 --- a/src/main/java/org/apache/commons/cli/DefaultParser.java +++ b/src/main/java/org/apache/commons/cli/DefaultParser.java @@ -518,16 +518,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(opt); + // the Options belong to the caller and outlive this parse, so hold the value on a copy + final Option copy = (Option) opt.clone(); + if (copy.hasArg() && copy.isValuesEmpty()) { + copy.processValue(stripLeadingAndTrailingQuotesDefaultOff(value)); + } + handleOption(copy); currentOption = null; } } diff --git a/src/main/java/org/apache/commons/cli/Parser.java b/src/main/java/org/apache/commons/cli/Parser.java index d120ea212..c1ef4374c 100644 --- a/src/main/java/org/apache/commons/cli/Parser.java +++ b/src/main/java/org/apache/commons/cli/Parser.java @@ -284,21 +284,22 @@ protected void processProperties(final Properties properties) throws ParseExcept if (!cmd.hasOption(option) && !selected) { // get the value from the properties instance final String value = properties.getProperty(option); - if (opt.hasArg()) { - if (opt.isValuesEmpty()) { - try { - opt.processValue(value); - } catch (final RuntimeException exp) { // NOPMD - // if we cannot add the value don't worry about it - } - } - } 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; } - cmd.addOption(opt); - updateRequiredOptions(opt); + // the Options belong to the caller and outlive this parse, so hold the value on a copy + final Option copy = (Option) opt.clone(); + if (copy.hasArg() && copy.isValuesEmpty()) { + try { + copy.processValue(value); + } catch (final RuntimeException exp) { // NOPMD + // if we cannot add the value don't worry about it + } + } + cmd.addOption(copy); + updateRequiredOptions(copy); } } } diff --git a/src/test/java/org/apache/commons/cli/AbstractParserTestCase.java b/src/test/java/org/apache/commons/cli/AbstractParserTestCase.java index 58657821a..a131046bf 100644 --- a/src/test/java/org/apache/commons/cli/AbstractParserTestCase.java +++ b/src/test/java/org/apache/commons/cli/AbstractParserTestCase.java @@ -616,6 +616,26 @@ void testPropertyOptionRequired() throws Exception { assertTrue(cmd.hasOption("f")); } + @Test + void testPropertyOptionReusedOptions() throws Exception { + final Options options = new Options(); + options.addOption(Option.builder("k").hasArg().get()); + + final Properties first = new Properties(); + first.setProperty("k", "one"); + final CommandLine firstCmd = parse(parser, options, null, first); + assertEquals("one", firstCmd.getOptionValue('k')); + + final Properties second = new Properties(); + second.setProperty("k", "two"); + final CommandLine secondCmd = parse(parser, options, null, second); + assertEquals("two", secondCmd.getOptionValue('k')); + + // the second parse must not reach back into the first result, and the Options must be left alone + assertEquals("one", firstCmd.getOptionValue('k')); + assertNull(options.getOption("k").getValue()); + } + @Test void testPropertyOptionSingularValue() throws Exception { final Options options = new Options();