Avoid NullPointerException in CommandLine.getOptionValues(Option|String)

Author: garydgregory

src/changes/changes.xml

@@ -30,6 +30,7 @@
       <action type="fix" issue="CLI-325" dev="ggregory" due-to="Claude Warren">Properties from multiple arguments with value separator. #237.</action>
       <action type="fix" issue="CLI-327" dev="ggregory" due-to="Claude Warren, Gary Gregory">Fix for expected textual date values. #244.</action>
       <action type="fix" dev="ggregory" due-to="Gary Gregory">Option.Builder.option("") should throw IllegalArgumentException instead of ArrayIndexOutOfBoundsException.</action>
+      <action type="fix" dev="ggregory" due-to="Gary Gregory">Avoid NullPointerException in CommandLine.getOptionValues(Option|String).</action>
       <!-- ADD -->
       <action type="add" issue="CLI-321" dev="ggregory" due-to="Claude Warren, Gary Gregory">Add and use a Converter interface and implementations without using BeanUtils #216.</action>
       <action type="add" dev="ggregory" due-to="Gary Gregory">Add Maven property project.build.outputTimestamp for build reproducibility.</action>

src/main/java/org/apache/commons/cli/CommandLine.java

@@ -179,6 +179,10 @@ protected void addOption(final Option opt) {
         }
     }
 
+    private <T> T get(final Supplier<T> supplier) {
+        return supplier == null ? null : supplier.get();
+    }
+
     /**
      * Gets any left-over non-recognized options and arguments
      *
@@ -317,9 +321,6 @@ public String getOptionValue(final char opt, final Supplier<String> defaultValue
      * @since 1.5.0
      */
     public String getOptionValue(final Option option) {
-        if (option == null) {
-            return null;
-        }
         final String[] values = getOptionValues(option);
         return values == null ? null : values[0];
     }
@@ -346,7 +347,7 @@ public String getOptionValue(final Option option, final String defaultValue) {
      */
     public String getOptionValue(final Option option, final Supplier<String> defaultValue) {
         final String answer = getOptionValue(option);
-        return answer != null ? answer : defaultValue.get();
+        return answer != null ? answer : get(defaultValue);
     }
 
     /**
@@ -382,6 +383,7 @@ public String getOptionValue(final String opt, final Supplier<String> defaultVal
         return getOptionValue(resolveOption(opt), defaultValue);
     }
 
+
     /**
      * Gets the array of values, if any, of an option.
      *
@@ -392,7 +394,6 @@ public String[] getOptionValues(final char opt) {
         return getOptionValues(String.valueOf(opt));
     }
 
-
     /**
      * Gets the array of values, if any, of an option.
      *
@@ -401,6 +402,9 @@ public String[] getOptionValues(final char opt) {
      * @since 1.5.0
      */
     public String[] getOptionValues(final Option option) {
+        if (option == null) {
+            return null;
+        }
         final List<String> values = new ArrayList<>();
         for (final Option processedOption : options) {
             if (processedOption.equals(option)) {
@@ -491,10 +495,13 @@ public <T> T getParsedOptionValue(final Option option) throws ParseException {
      */
     @SuppressWarnings("unchecked")
     public <T> T getParsedOptionValue(final Option option, final Supplier<T> defaultValue) throws ParseException {
-        final String res = option == null ? null : getOptionValue(option);
+        if (option == null) {
+            return get(defaultValue);
+        }
+        final String res = getOptionValue(option);
         try {
             if (res == null) {
-                return defaultValue == null ? null : defaultValue.get();
+                return get(defaultValue);
             }
             return (T) option.getConverter().apply(res);
         } catch (final Throwable e) {

src/main/java/org/apache/commons/cli/HelpFormatter.java

@@ -19,7 +19,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 
 import java.io.BufferedReader;
 import java.io.IOException;
-import java.io.PrintStream;
 import java.io.PrintWriter;
 import java.io.Serializable;
 import java.io.StringReader;

src/test/java/org/apache/commons/cli/ValueTest.java

@@ -27,8 +27,12 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 
 @SuppressWarnings("deprecation") // tests some deprecated classes
 public class ValueTest {
-    private CommandLine cl;
+
+    private static final Option NULL_OPTION = null;
+    private static final String NULL_STRING = null;
+
     private final Options opts = new Options();
+    private CommandLine cl;
 
     @BeforeEach
     public void setUp() throws Exception {
@@ -78,6 +82,8 @@ public void testLongOptionalArgValues() throws Exception {
 
         final Parser parser = new PosixParser();
         final CommandLine cmd = parser.parse(opts, args);
+        assertNull(cmd.getOptionValues(NULL_OPTION));
+        assertNull(cmd.getOptionValues(NULL_STRING));
         assertTrue(cmd.hasOption("gravy"));
         assertEquals("gold", cmd.getOptionValue("gravy"));
         assertEquals("gold", cmd.getOptionValues("gravy")[0]);
@@ -91,6 +97,8 @@ public void testLongOptionalArgValuesWithOption() throws Exception {
 
         final Parser parser = new PosixParser();
         final CommandLine cmd = parser.parse(opts, args);
+        assertNull(cmd.getOptionValues(NULL_OPTION));
+        assertNull(cmd.getOptionValues(NULL_STRING));
         assertTrue(cmd.hasOption(opts.getOption("gravy")));
         assertEquals("gold", cmd.getOptionValue(opts.getOption("gravy")));
         assertEquals("gold", cmd.getOptionValues(opts.getOption("gravy"))[0]);
@@ -130,6 +138,8 @@ public void testLongOptionalNArgValuesWithOption() throws Exception {
         final Parser parser = new PosixParser();
 
         final CommandLine cmd = parser.parse(opts, args);
+        assertNull(cmd.getOptionValues(NULL_OPTION));
+        assertNull(cmd.getOptionValues(NULL_STRING));
         assertTrue(cmd.hasOption(opts.getOption("hide")));
         assertEquals("house", cmd.getOptionValue(opts.getOption("hide")));
         assertEquals("house", cmd.getOptionValues(opts.getOption("hide"))[0]);
@@ -233,6 +243,8 @@ public void testShortOptionalArgValuesWithOption() throws Exception {
 
         final Parser parser = new PosixParser();
         final CommandLine cmd = parser.parse(opts, args);
+        assertNull(cmd.getOptionValues(NULL_OPTION));
+        assertNull(cmd.getOptionValues(NULL_STRING));
         assertTrue(cmd.hasOption(opts.getOption("j")));
         assertEquals("ink", cmd.getOptionValue(opts.getOption("j")));
         assertEquals("ink", cmd.getOptionValues(opts.getOption("j"))[0]);
@@ -271,6 +283,8 @@ public void testShortOptionalNArgValuesWithOption() throws Exception {
 
         final Parser parser = new PosixParser();
         final CommandLine cmd = parser.parse(opts, args);
+        assertNull(cmd.getOptionValues(NULL_OPTION));
+        assertNull(cmd.getOptionValues(NULL_STRING));
         assertTrue(cmd.hasOption("i"));
         assertEquals("ink", cmd.getOptionValue(opts.getOption("i")));
         assertEquals("ink", cmd.getOptionValues(opts.getOption("i"))[0]);