[
https://issues.apache.org/jira/browse/CLI-342?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17985218#comment-17985218
]
Ruiqi Dong commented on CLI-342:
--------------------------------
Do deprecated classes need to report bugs? If the answer is no, I will stop
reporting bugs on those deprecated classes.
> Parser fails to throw MissingArgumentException for empty required arguments
> when parsing via Properties
> -------------------------------------------------------------------------------------------------------
>
> Key: CLI-342
> URL: https://issues.apache.org/jira/browse/CLI-342
> Project: Commons CLI
> Issue Type: Bug
> Components: Parser
> Affects Versions: 1.10.0
> Reporter: Ruiqi Dong
> Priority: Major
> Fix For: 1.10.0
>
> Attachments: Parser.java, ParserTest.java
>
> Original Estimate: 0.5h
> Remaining Estimate: 0.5h
>
> The {{Parser}} class silently ignores invalid/empty argument values when
> parsing options through Properties, while the same scenario correctly throws
> {{MissingArgumentException}} when parsing via command line arguments. This
> inconsistent behavior is caused by a catch block that swallows
> {{RuntimeException}} in the {{processProperties}} method.
>
> *Description:*
> When parsing options that require arguments through Properties, if an empty
> or invalid value is provided, the parser does not throw a
> {{MissingArgumentException}} as expected. Instead, the exception is silently
> swallowed due to a catch block in the {{processProperties}} method. This
> creates inconsistent behavior between command-line parsing and
> Properties-based parsing.
>
> *Steps to Reproduce:*
> 1. Create an Option that requires an argument:
> Options options = new Options();
> Option option = new Option("f", "file", true, "Input file");
> options.addOption(option);
>
> 2. Parse via Properties with empty value:
> Properties properties = new Properties();
> properties.setProperty("file", "");
> Parser parser = new PosixParser();
> CommandLine cmd = parser.parse(options, new String[0], properties);
>
> 3. No exception is thrown, but the option has no valid value
>
> *Expected Behavior:* The parser should throw a {{MissingArgumentException}}
> when a required argument is missing or empty, consistent with command-line
> parsing behavior.
> *Actual Behavior:* No exception is thrown. The RuntimeException is caught and
> silently ignored in the {{processProperties}} method.
>
> *Unit Test Case:*
> @Test
> void testEmptyArgumentViaPropertiesShouldThrowException() {
> Options options = new Options();
> Option option = new Option("f", "file", true, "Input file");
> options.addOption(option);
>
> Properties properties = new Properties();
> properties.setProperty("file", "");
>
> Parser parser = new PosixParser();
>
> // This should throw ParseException but doesn't
> assertThrows(ParseException.class, () ->
> { parser.parse(options, new String[0], properties); }
> );
> }
>
> *Test Results:*
> [*INFO*] Running org.apache.commons.cli.{*}ParserTest{*}{*}{*}
> [*ERROR*] *Tests* {*}run: 3{*}, *Failures: 1*, Errors: 0, Skipped: 0, Time
> elapsed: 0.009 s *<<< FAILURE!* -- in
> org.apache.commons.cli.{*}ParserTest{*}{*}{*}
> [*ERROR*]
> org.apache.commons.cli.ParserTest.testEmptyArgumentViaPropertiesShouldThrowException
> -- Time elapsed: 0 s <<< FAILURE!
> org.opentest4j.AssertionFailedError: should throw ParseException when
> required argument value is empty string ==> Expected
> org.apache.commons.cli.ParseException to be thrown, but nothing was thrown.
> at
> org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:152)
> at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:73)
> at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:39)
> at org.junit.jupiter.api.Assertions.assertThrows(Assertions.java:3153)
> at
> org.apache.commons.cli.ParserTest.testEmptyArgumentViaPropertiesShouldThrowException(ParserTest.java:50)
> at java.lang.reflect.Method.invoke(Method.java:498)
> at java.util.ArrayList.forEach(ArrayList.java:1259)
> at java.util.ArrayList.forEach(ArrayList.java:1259)
>
> *Root Cause Analysis:*
> In {{{}Parser.java{}}}, the {{processProperties}} method contains:
> if (opt.hasArg()) {
> if (Util.isEmpty(opt.getValues())) {
> try
> { opt.processValue(value); }
> catch (final RuntimeException exp) \{ // NOPMD // if we cannot
> add the value don't worry about it }
> }
> }
> The catch block silently swallows any {{RuntimeException}} thrown by
> {{{}processValue(){}}}, including cases where the value is invalid or empty.
> When parsing via command line, the {{processArgs}} method correctly handles
> this:
> if (opt.getValues() == null && !opt.hasOptionalArg()) \{ throw new
> MissingArgumentException(opt); }
>
> *Suggested Fix:*
> Remove or modify the exception handling in {{processProperties}} to maintain
> consistency with command-line parsing:
> if (opt.hasArg()) {
> if (Util.isEmpty(opt.getValues()))
> { opt.processValue(value); // Check if value was successfully
> set if (opt.getValues() == null && !opt.hasOptionalArg()) \\{
> throw new MissingArgumentException(opt); }
> }
> }
> Alternatively, catch and re-throw as ParseException:
> if (opt.hasArg()) {
> if (Util.isEmpty(opt.getValues()))
> { try \\{ opt.processValue(value); }
> catch (final RuntimeException exp)
> { throw new ParseException("Invalid value for option: " + option
> + " - " + exp.getMessage()); }
> // Validate the value was set
> if (opt.getValues() == null && !opt.hasOptionalArg())
> { throw new MissingArgumentException(opt); }
> }
> }
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
