From 8bbdbc9d3365c59d8e89647974e2d8f5c9bc0c6c Mon Sep 17 00:00:00 2001 From: farkhalit rida Date: Tue, 21 Jul 2026 12:57:55 +0530 Subject: [PATCH 1/3] copy the Option before applying property values in handleProperties --- .../org/apache/commons/cli/DefaultParser.java | 11 +++++----- .../java/org/apache/commons/cli/Parser.java | 12 ++++++----- .../commons/cli/AbstractParserTestCase.java | 20 +++++++++++++++++++ 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java index 9f9285d78..dd1523c23 100644 --- a/src/main/java/org/apache/commons/cli/DefaultParser.java +++ b/src/main/java/org/apache/commons/cli/DefaultParser.java @@ -518,16 +518,17 @@ 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)); + // 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()) { + if (copy.isValuesEmpty()) { + copy.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; } - handleOption(opt); + 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..66e7d96ec 100644 --- a/src/main/java/org/apache/commons/cli/Parser.java +++ b/src/main/java/org/apache/commons/cli/Parser.java @@ -284,10 +284,12 @@ 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()) { + // 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()) { + if (copy.isValuesEmpty()) { try { - opt.processValue(value); + copy.processValue(value); } catch (final RuntimeException exp) { // NOPMD // if we cannot add the value don't worry about it } @@ -297,8 +299,8 @@ protected void processProperties(final Properties properties) throws ParseExcept // option to the CommandLine continue; } - cmd.addOption(opt); - updateRequiredOptions(opt); + 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(); From 51384fc611705781e6bd2414bb950cde69167aae Mon Sep 17 00:00:00 2001 From: farkhalit rida Date: Wed, 29 Jul 2026 12:40:10 +0530 Subject: [PATCH 2/3] Defer the Option clone until after the no-arg skip check Signed-off-by: farkhalit rida --- .../org/apache/commons/cli/DefaultParser.java | 13 ++++++------ .../java/org/apache/commons/cli/Parser.java | 21 +++++++++---------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java index dd1523c23..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,15 @@ 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); - // 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()) { - if (copy.isValuesEmpty()) { - copy.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; } + // 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 66e7d96ec..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,20 @@ 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); - // 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()) { - if (copy.isValuesEmpty()) { - try { - copy.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; } + // 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); } From 3dda9d239ebe9232dd11ed414287d80faf56bad7 Mon Sep 17 00:00:00 2001 From: farkhalit rida Date: Tue, 6 Oct 2026 04:37:48 +0530 Subject: [PATCH 3/3] Set the property value on the copy that handleOption already makes handleProperties cloned the Option itself and then passed the clone to handleOption, which clones again. Call handleOption first and store the value on currentOption, the copy it just made, which is the same idiom the argv paths use and leaves one clone per property option. --- src/main/java/org/apache/commons/cli/DefaultParser.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/apache/commons/cli/DefaultParser.java b/src/main/java/org/apache/commons/cli/DefaultParser.java index e80edc93d..0820c18bd 100644 --- a/src/main/java/org/apache/commons/cli/DefaultParser.java +++ b/src/main/java/org/apache/commons/cli/DefaultParser.java @@ -509,12 +509,11 @@ private void handleProperties(final Properties properties) throws ParseException // if the value is not yes, true or 1 then don't add the option to the CommandLine continue; } - // 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(opt); + // the Options belong to the caller and outlive this parse, so hold the value on the copy made by handleOption + if (currentOption != null && currentOption.isValuesEmpty()) { + currentOption.processValue(stripLeadingAndTrailingQuotesDefaultOff(value)); } - handleOption(copy); currentOption = null; } }