This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-cli.git
The following commit(s) were added to refs/heads/master by this push:
new e04b9bc2 Fix DefaultParser keeping property values across parses (#450)
e04b9bc2 is described below
commit e04b9bc29d1a2c3c203b7cd81235fcac9c4ec516
Author: Andrea Cosentino <[email protected]>
AuthorDate: Thu Oct 29 01:58:47 2026 +0100
Fix DefaultParser keeping property values across parses (#450)
* 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 <[email protected]>
Signed-off-by: Andrea Cosentino <[email protected]>
* 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 <[email protected]>
Signed-off-by: Andrea Cosentino <[email protected]>
---------
Signed-off-by: Andrea Cosentino <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
src/changes/changes.xml | 1 +
.../java/org/apache/commons/cli/DefaultParser.java | 19 +++++++---
.../org/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 d3f7b92d..844f21b7 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -36,6 +36,7 @@
<action type="fix" dev="ggregory" due-to="Naveed Khan, Gary Gregory">Fix
integer overflow in TextHelpAppendable.indexOfWrap (#437).</action>
<action type="fix" dev="ggregory" due-to="Naveed Khan, Gary
Gregory">Reject UTF-16 surrogate values in Character converter (#438).</action>
<action type="fix" dev="ggregory" due-to="dev_Hakaze, Gary Gregory"
issue="CLI-354">Fix HelpFormatter wrapped description indent (#439).</action>
+ <action type="fix" dev="acosentino" due-to="Andrea Cosentino">Fix
DefaultParser keeping values from its Properties argument across parses that
reuse the same Options.</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory, farkhalit
rida">Load classes without initializing them in Converter.CLASS (#432).</action>
<!-- ADD -->
<action type="fix" dev="ggregory" due-to="Elric, Gary Gregory">Add
AbstractHelpFormatter.printHelp(String, Options).</action>
diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java
b/src/main/java/org/apache/commons/cli/DefaultParser.java
index 4dd7dfec..0060e3e8 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 @@ public class DefaultParser implements CommandLineParser {
// 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 @@ public class DefaultParser implements CommandLineParser {
/**
* Parses the arguments according to the specified options and properties.
+ * <p>
+ * 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.
+ * </p>
*
* @param options The specified Options.
* @param properties command line option name-value pairs.
@@ -790,6 +796,7 @@ public class DefaultParser implements CommandLineParser {
* @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 eb21171f..0e7782fb 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 @@ package org.apache.commons.cli;
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 @@ class DefaultParserTest extends AbstractParserTestCase {
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 @@ class DefaultParserTest extends AbstractParserTestCase {
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\"")
