From d19562cf7721092a5ec6a334ea65ab080a8c8b92 Mon Sep 17 00:00:00 2001 From: Andrea Cosentino Date: Wed, 28 Oct 2026 14:20:11 +0100 Subject: [PATCH 1/2] Fix DefaultParser keeping property values across parses When parse() gets a Properties object, handleProperties() called processValue() on the Option registered in Options instead of on the copy that goes into the CommandLine, and only when that Option had no values yet. Nothing ever clears it, so once you've parsed with properties, reusing the same Options gives stale results: a new property value is ignored, a value given on the command line is replaced by the old one and ends up in the argument list, and pairs from a "-D" style option carry over into the next parse. Add the value to the copy instead, like values from the command line, so parsing doesn't change the Options anymore. A value the Option already has still wins over the property, as before, and the Javadoc now says so. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Andrea Cosentino --- src/changes/changes.xml | 1 + .../org/apache/commons/cli/DefaultParser.java | 19 +++++--- .../apache/commons/cli/DefaultParserTest.java | 44 +++++++++++++++++++ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index d3f7b92d2..844f21b79 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -36,6 +36,7 @@ Fix integer overflow in TextHelpAppendable.indexOfWrap (#437). Reject UTF-16 surrogate values in Character converter (#438). Fix HelpFormatter wrapped description indent (#439). + Fix DefaultParser keeping values from its Properties argument across parses that reuse the same Options. Load classes without initializing them in Converter.CLASS (#432). Add AbstractHelpFormatter.printHelp(String, Options). diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java index 4dd7dfec2..3ed18aed0 100644 --- a/src/main/java/org/apache/commons/cli/DefaultParser.java +++ b/src/main/java/org/apache/commons/cli/DefaultParser.java @@ -506,14 +506,16 @@ private void handleProperties(final Properties properties) throws ParseException // get the value from the properties final String value = properties.getProperty(option); if (opt.hasArg()) { - if (opt.isValuesEmpty()) { - opt.processValue(stripLeadingAndTrailingQuotesDefaultOff(value)); + // add the value to the copy of the option in the CommandLine, not to the option registered in Options, + // and keep a value the option already has + handleOption(opt); + if (currentOption.isValuesEmpty()) { + currentOption.processValue(stripLeadingAndTrailingQuotesDefaultOff(value)); } - } else if (!("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; + } else if ("yes".equalsIgnoreCase(value) || "true".equalsIgnoreCase(value) || "1".equalsIgnoreCase(value)) { + // only add the option to the CommandLine if the value is yes, true or 1 + handleOption(opt); } - handleOption(opt); currentOption = null; } } @@ -727,6 +729,10 @@ private boolean isShortOption(final String token) { /** * Parses the arguments according to the specified options and properties. + *

+ * A value from {@code properties} is only used for an option that is not on the command line and has no value yet. It goes into the returned + * {@link CommandLine}; {@code options} itself is not modified. + *

* * @param options The specified Options. * @param properties command line option name-value pairs. @@ -790,6 +796,7 @@ public CommandLine parse(final Options options, final String[] arguments, final * @param properties command line option name-value pairs. * @return The list of atomic option and value tokens. * @throws ParseException if there are any problems encountered while parsing the command line tokens. + * @see #parse(Options, Properties, NonOptionAction, String[]) */ public CommandLine parse(final Options options, final String[] arguments, final Properties properties) throws ParseException { return parse(options, arguments, properties, false); diff --git a/src/test/java/org/apache/commons/cli/DefaultParserTest.java b/src/test/java/org/apache/commons/cli/DefaultParserTest.java index eb21171fd..0e7782fbe 100644 --- a/src/test/java/org/apache/commons/cli/DefaultParserTest.java +++ b/src/test/java/org/apache/commons/cli/DefaultParserTest.java @@ -19,6 +19,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -401,6 +402,36 @@ void testParseSkipNonHappyPath() throws ParseException { assertTrue(e.getMessage().contains("-d")); } + @Test + void testPropertiesDoNotChangeRegisteredOption() throws ParseException { + final Options options = new Options(); + options.addOption(Option.builder("o").hasArg().get()); + final Properties properties = new Properties(); + properties.setProperty("o", "fromProperties"); + final CommandLine cmd = new DefaultParser().parse(options, new String[0], properties); + assertEquals("fromProperties", cmd.getOptionValue("o")); + assertNull(options.getOption("o").getValues()); + } + + @Test + void testPropertiesDoNotLeakIntoNextParse() throws ParseException { + final Options options = new Options(); + options.addOption(Option.builder("o").hasArg().get()); + options.addOption(Option.builder("D").hasArgs().valueSeparator('=').get()); + final Properties properties = new Properties(); + properties.setProperty("o", "fromProperties"); + properties.setProperty("D", "key1=value1"); + final DefaultParser defaultParser = new DefaultParser(); + defaultParser.parse(options, new String[0], properties); + // parse again with the same Options and without properties + final CommandLine cmd = defaultParser.parse(options, new String[] { "-o", "fromArgs", "-Dkey2=value2" }); + assertEquals("fromArgs", cmd.getOptionValue("o")); + assertTrue(cmd.getArgList().isEmpty()); + final Properties expected = new Properties(); + expected.setProperty("key2", "value2"); + assertEquals(expected, cmd.getOptionProperties("D")); + } + @Test void testPropertiesPreserveExistingOptionValue() throws ParseException { final Option option = Option.builder("o").hasArg().get(); @@ -415,6 +446,19 @@ void testPropertiesPreserveExistingOptionValue() throws ParseException { assertEquals("preset", option.getValue()); } + @Test + void testPropertiesReuseOptions() throws ParseException { + final Options options = new Options(); + options.addOption(Option.builder("o").hasArg().get()); + final DefaultParser defaultParser = new DefaultParser(); + final Properties first = new Properties(); + first.setProperty("o", "first"); + assertEquals("first", defaultParser.parse(options, new String[0], first).getOptionValue("o")); + final Properties second = new Properties(); + second.setProperty("o", "second"); + assertEquals("second", defaultParser.parse(options, new String[0], second).getOptionValue("o")); + } + @Override @Test @Disabled("Test case handled in the parameterized tests as \"DEFAULT behavior\"") From f5809722f818c38b6264afd31ad179ec0609e5df Mon Sep 17 00:00:00 2001 From: Andrea Cosentino Date: Wed, 28 Oct 2026 15:17:33 +0100 Subject: [PATCH 2/2] Javadoc: Narrow the note about property values Parsing still updates the option group selections, so saying that options itself isn't modified went too far. Say what actually holds: property values go into the returned CommandLine without changing the values of the registered options. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Andrea Cosentino --- src/main/java/org/apache/commons/cli/DefaultParser.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java index 3ed18aed0..0060e3e80 100644 --- a/src/main/java/org/apache/commons/cli/DefaultParser.java +++ b/src/main/java/org/apache/commons/cli/DefaultParser.java @@ -730,8 +730,8 @@ private boolean isShortOption(final String token) { /** * Parses the arguments according to the specified options and properties. *

- * A value from {@code properties} is only used for an option that is not on the command line and has no value yet. It goes into the returned - * {@link CommandLine}; {@code options} itself is not modified. + * A value from {@code properties} is only used for an option that is not on the command line and has no value yet. Property values are added to the + * returned {@link CommandLine} without changing the values of the registered options. *

* * @param options The specified Options.