[jira] [Updated] (CLI-344) Option.processValue() throws NullPointerException when passed null value with value separator configured

["Gary D. Gregory (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/CLI-344?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gary D. Gregory updated CLI-344:
--------------------------------
    Priority: Minor  (was: Major)

> Option.processValue() throws NullPointerException when passed null value with value separator configured
> --------------------------------------------------------------------------------------------------------
>
>                 Key: CLI-344
>                 URL: https://issues.apache.org/jira/browse/CLI-344
>             Project: Commons CLI
>          Issue Type: Bug
>          Components: Options definition
>    Affects Versions: 1.10.0
>            Reporter: Ruiqi Dong
>            Assignee: Gary D. Gregory
>            Priority: Minor
>             Fix For: 1.10.0
>
>         Attachments: Option.java, OptionTest.java
>
>   Original Estimate: 0.5h
>  Remaining Estimate: 0.5h
>
> The {{Option.processValue(String value)}} method does not properly handle null input values. When a null value is passed to this method and the Option has a value separator configured, it throws a {{NullPointerException}} instead of handling the null gracefully with proper validation.
>  
> *Steps to Reproduce:*
> Option option = new Option("D", true, "Define property");
> option.setValueSeparator('=');
> option.processValue(null);  // Throws NullPointerException
>  
> *Expected Behavior:* The method should validate the input and throw an {{IllegalArgumentException}} with a clear error message when null is passed, consistent with how other methods in the library handle invalid input.
> *Actual Behavior:* A {{NullPointerException}} is thrown at line 839 in Option.java when the code attempts to call {{indexOf()}} on the null value:
> java.lang.NullPointerException
>     at org.apache.commons.cli.Option.processValue(Option.java:839)
>  
> *Root Cause Analysis:*
> The {{processValue()}} method assigns the input value directly to a local variable without null checking:
> void processValue(final String value) {
>     if (argCount == UNINITIALIZED) {
>         throw new IllegalArgumentException("NO_ARGS_ALLOWED");
>     }
>     String add = value;  // No null check
>     if (hasValueSeparator()) {
>         final char sep = getValueSeparator();
>         int index = add.indexOf(sep);  // NPE occurs here when add is null
>         // ...
>     }
> }
>  
> *Unit Test Cases:*
> @Test
> public void testProcessValueWithNullThrowsIllegalArgumentException() {
>     Option option = new Option("D", true, "Define property");
>     option.setValueSeparator('=');
>     
>     IllegalArgumentException exception = assertThrows(
>         IllegalArgumentException.class, 
>         () -> option.processValue(null),
>         "processValue(null) should throw IllegalArgumentException"
>     );
>     
>     assertTrue(exception.getMessage().contains("null"),
>         "Exception message should indicate null value issue");
> }
>  
> *Test Results:*
> [*INFO*] Running org.apache.commons.cli.{*}OptionTest{*}{*}{*}
> [*ERROR*] *Tests* {*}run: 1{*}, *Failures: 1*, Errors: 0, Skipped: 0, Time elapsed: 0.001 s *<<< FAILURE!* -- in org.apache.commons.cli.{*}OptionTest{*}{*}{*}
> [*ERROR*] org.apache.commons.cli.OptionTest.testProcessValueWithNullThrowsIllegalArgumentException -- Time elapsed: 0.001 s <<< FAILURE!
> org.opentest4j.AssertionFailedError: processValue(null) should throw IllegalArgumentException ==> Unexpected exception type thrown, expected: <java.lang.IllegalArgumentException> but was: <java.lang.NullPointerException>
>  at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
>  at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:67)
>  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.OptionTest.testProcessValueWithNullThrowsIllegalArgumentException(OptionTest.java:32)
>  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)
> Caused by: java.lang.NullPointerException
>  at org.apache.commons.cli.Option.processValue(Option.java:839)
>  at org.apache.commons.cli.OptionTest.lambda$testProcessValueWithNullThrowsIllegalArgumentException$0(OptionTest.java:34)
>  at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:53)
>  ... 6 more
>  
> *Suggested Fix:* 
> Add null validation at the beginning of the method:
> void processValue(final String value) {
>     if (value == null) {
>         throw new IllegalArgumentException("The value must not be null");
>     }
>     if (argCount == UNINITIALIZED) {
>         throw new IllegalArgumentException("NO_ARGS_ALLOWED");
>     }
>     // ... rest of the method remains unchanged
> }



--
This message was sent by Atlassian Jira
(v8.20.10#820010)