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

[Ruiqi Dong (Jira)]

Ruiqi Dong created CLI-344:
------------------------------

             Summary: 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
    Affects Versions: 1.10.0
            Reporter: Ruiqi Dong
             Fix For: 1.10.0
         Attachments: Option.java, OptionTest.java

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)