Fix regression for null varargs in SpEL expressions

A regression was introduced in gh-27582. Specifically, when null is
supplied as the single argument for a varargs parameter in a method or
function in a SpEL expression, ReflectionHelper currently throws a
NullPointerException instead of leaving the null value unchanged.

This commit fixes this regression.

Closes gh-27719

Author: sbrannen

spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java

@@ -290,17 +290,18 @@ static boolean convertArguments(TypeConverter converter, Object[] arguments, Exe
 				Object argument = arguments[varargsPosition];
 				TypeDescriptor targetType = new TypeDescriptor(methodParam);
 				TypeDescriptor sourceType = TypeDescriptor.forObject(argument);
-				// If the argument type is equal to the varargs element type, there is no need
-				// to convert it or wrap it in an array. For example, using StringToArrayConverter
-				// to convert a String containing a comma would result in the String being split
-				// and repackaged in an array when it should be used as-is.
-				if (!sourceType.equals(targetType.getElementTypeDescriptor())) {
+				// If the argument is null or the argument type is equal to the varargs element type,
+				// there is no need to convert it or wrap it in an array. For example, using
+				// StringToArrayConverter to convert a String containing a comma would result in the
+				// String being split and repackaged in an array when it should be used as-is.
+				if (argument != null && !sourceType.equals(targetType.getElementTypeDescriptor())) {
 					arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType);
 				}
-				// Three outcomes of the above if-block:
-				// 1) the input argument was correct type but not wrapped in an array, and nothing was done.
-				// 2) the input argument was already compatible (i.e., array of valid type), and nothing was done.
-				// 3) the input argument was the wrong type and got converted and wrapped in an array.
+				// Possible outcomes of the above if-block:
+				// 1) the input argument was null, and nothing was done.
+				// 2) the input argument was correct type but not wrapped in an array, and nothing was done.
+				// 3) the input argument was already compatible (i.e., array of valid type), and nothing was done.
+				// 4) the input argument was the wrong type and got converted and wrapped in an array.
 				if (argument != arguments[varargsPosition] &&
 						!isFirstEntryInArray(argument, arguments[varargsPosition])) {
 					conversionOccurred = true; // case 3

spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java

@@ -234,26 +234,33 @@ public void testAddingMethodResolvers() {
 
 	@Test
 	public void testVarargsInvocation01() {
-		// Calling 'public int aVarargsMethod(String... strings)' - returns number of arguments
-		evaluate("aVarargsMethod('a','b','c')", 3, Integer.class);
-		evaluate("aVarargsMethod('a')", 1, Integer.class);
-		evaluate("aVarargsMethod()", 0, Integer.class);
-		evaluate("aVarargsMethod(1,2,3)", 3, Integer.class); // all need converting to strings
-		evaluate("aVarargsMethod(1)", 1, Integer.class); // needs string conversion
-		evaluate("aVarargsMethod(1,'a',3.0d)", 3, Integer.class); // first and last need conversion
-		evaluate("aVarargsMethod(new String[]{'a','b','c'})", 3, Integer.class);
+		// Calling 'public String aVarargsMethod(String... strings)'
+		evaluate("aVarargsMethod('a','b','c')", "[a, b, c]", String.class);
+		evaluate("aVarargsMethod('a')", "[a]", String.class);
+		evaluate("aVarargsMethod()", "[]", String.class);
+		evaluate("aVarargsMethod(1,2,3)", "[1, 2, 3]", String.class); // all need converting to strings
+		evaluate("aVarargsMethod(1)", "[1]", String.class); // needs string conversion
+		evaluate("aVarargsMethod(1,'a',3.0d)", "[1, a, 3.0]", String.class); // first and last need conversion
+		evaluate("aVarargsMethod(new String[]{'a','b','c'})", "[a, b, c]", String.class);
+		evaluate("aVarargsMethod(new String[]{})", "[]", String.class);
+		evaluate("aVarargsMethod(null)", "[null]", String.class);
+		evaluate("aVarargsMethod(null,'a')", "[null, a]", String.class);
+		evaluate("aVarargsMethod('a',null,'b')", "[a, null, b]", String.class);
 	}
 
 	@Test
 	public void testVarargsInvocation02() {
-		// Calling 'public int aVarargsMethod2(int i, String... strings)' - returns int + length_of_strings
-		evaluate("aVarargsMethod2(5,'a','b','c')", 8, Integer.class);
-		evaluate("aVarargsMethod2(2,'a')", 3, Integer.class);
-		evaluate("aVarargsMethod2(4)", 4, Integer.class);
-		evaluate("aVarargsMethod2(8,2,3)", 10, Integer.class);
-		evaluate("aVarargsMethod2(9)", 9, Integer.class);
-		evaluate("aVarargsMethod2(2,'a',3.0d)", 4, Integer.class);
-		evaluate("aVarargsMethod2(8,new String[]{'a','b','c'})", 11, Integer.class);
+		// Calling 'public String aVarargsMethod2(int i, String... strings)'
+		evaluate("aVarargsMethod2(5,'a','b','c')", "5-[a, b, c]", String.class);
+		evaluate("aVarargsMethod2(2,'a')", "2-[a]", String.class);
+		evaluate("aVarargsMethod2(4)", "4-[]", String.class);
+		evaluate("aVarargsMethod2(8,2,3)", "8-[2, 3]", String.class);
+		evaluate("aVarargsMethod2(2,'a',3.0d)", "2-[a, 3.0]", String.class);
+		evaluate("aVarargsMethod2(8,new String[]{'a','b','c'})", "8-[a, b, c]", String.class);
+		evaluate("aVarargsMethod2(8,new String[]{})", "8-[]", String.class);
+		evaluate("aVarargsMethod2(8,null)", "8-[null]", String.class);
+		evaluate("aVarargsMethod2(8,null,'a')", "8-[null, a]", String.class);
+		evaluate("aVarargsMethod2(8,'a',null,'b')", "8-[a, null, b]", String.class);
 	}
 
 	@Test
@@ -284,6 +291,25 @@ public void testVarargsInvocation03() {
 		evaluate("aVarargsMethod3('foo', 'bar,baz')", "foo-bar,baz", String.class);
 	}
 
+	@Test
+	public void testVarargsOptionalInvocation() {
+		// Calling 'public String optionalVarargsMethod(Optional<String>... values)'
+		evaluate("optionalVarargsMethod()", "[]", String.class);
+		evaluate("optionalVarargsMethod(new String[0])", "[]", String.class);
+		evaluate("optionalVarargsMethod('a')", "[Optional[a]]", String.class);
+		evaluate("optionalVarargsMethod('a','b','c')", "[Optional[a], Optional[b], Optional[c]]", String.class);
+		evaluate("optionalVarargsMethod(9)", "[Optional[9]]", String.class);
+		evaluate("optionalVarargsMethod(2,3)", "[Optional[2], Optional[3]]", String.class);
+		evaluate("optionalVarargsMethod('a',3.0d)", "[Optional[a], Optional[3.0]]", String.class);
+		evaluate("optionalVarargsMethod(new String[]{'a','b','c'})", "[Optional[a], Optional[b], Optional[c]]", String.class);
+		// The following should actually evaluate to [Optional.empty] instead of [null],
+		// but ReflectionHelper.convertArguments() currently does not provide explicit
+		// Optional support for a single argument passed to a varargs array.
+		evaluate("optionalVarargsMethod(null)", "[null]", String.class);
+		evaluate("optionalVarargsMethod(null,'a')", "[Optional.empty, Optional[a]]", String.class);
+		evaluate("optionalVarargsMethod('a',null,'b')", "[Optional[a], Optional.empty, Optional[b]]", String.class);
+	}
+
 	@Test
 	public void testInvocationOnNullContextObject() {
 		evaluateAndCheckError("null.toString()",SpelMessage.METHOD_CALL_ON_NULL_OBJECT_NOT_ALLOWED);

spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
 
 package org.springframework.expression.spel;
 
+import java.util.Arrays;
 import java.util.GregorianCalendar;
 
 import org.springframework.expression.spel.support.StandardEvaluationContext;
@@ -51,10 +52,10 @@ private static void populateFunctions(StandardEvaluationContext testContext) {
 					TestScenarioCreator.class.getDeclaredMethod("reverseInt", Integer.TYPE, Integer.TYPE, Integer.TYPE));
 			testContext.registerFunction("reverseString",
 					TestScenarioCreator.class.getDeclaredMethod("reverseString", String.class));
-			testContext.registerFunction("varargsFunctionReverseStringsAndMerge",
-					TestScenarioCreator.class.getDeclaredMethod("varargsFunctionReverseStringsAndMerge", String[].class));
-			testContext.registerFunction("varargsFunctionReverseStringsAndMerge2",
-					TestScenarioCreator.class.getDeclaredMethod("varargsFunctionReverseStringsAndMerge2", Integer.TYPE, String[].class));
+			testContext.registerFunction("varargsFunction",
+					TestScenarioCreator.class.getDeclaredMethod("varargsFunction", String[].class));
+			testContext.registerFunction("varargsFunction2",
+					TestScenarioCreator.class.getDeclaredMethod("varargsFunction2", Integer.TYPE, String[].class));
 		}
 		catch (Exception ex) {
 			throw new IllegalStateException(ex);
@@ -108,25 +109,12 @@ public static String reverseString(String input) {
 		return backwards.toString();
 	}
 
-	public static String varargsFunctionReverseStringsAndMerge(String... strings) {
-		StringBuilder sb = new StringBuilder();
-		if (strings != null) {
-			for (int i = strings.length - 1; i >= 0; i--) {
-				sb.append(strings[i]);
-			}
-		}
-		return sb.toString();
+	public static String varargsFunction(String... strings) {
+		return Arrays.toString(strings);
 	}
 
-	public static String varargsFunctionReverseStringsAndMerge2(int j, String... strings) {
-		StringBuilder sb = new StringBuilder();
-		sb.append(j);
-		if (strings != null) {
-			for (int i = strings.length - 1; i >= 0; i--) {
-				sb.append(strings[i]);
-			}
-		}
-		return sb.toString();
+	public static String varargsFunction2(int i, String... strings) {
+		return String.valueOf(i) + "-" + Arrays.toString(strings);
 	}
 
 }

spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java

@@ -59,21 +59,33 @@ public void testFunctionAccess02() {
 
 	@Test
 	public void testCallVarargsFunction() {
-		evaluate("#varargsFunctionReverseStringsAndMerge('a,b')", "a,b", String.class);
-		evaluate("#varargsFunctionReverseStringsAndMerge('a', 'b,c', 'd')", "db,ca", String.class);
-		evaluate("#varargsFunctionReverseStringsAndMerge('a','b','c')", "cba", String.class);
-		evaluate("#varargsFunctionReverseStringsAndMerge('a')", "a", String.class);
-		evaluate("#varargsFunctionReverseStringsAndMerge()", "", String.class);
-		evaluate("#varargsFunctionReverseStringsAndMerge('b',25)", "25b", String.class);
-		evaluate("#varargsFunctionReverseStringsAndMerge(25)", "25", String.class);
+		evaluate("#varargsFunction()", "[]", String.class);
+		evaluate("#varargsFunction(new String[0])", "[]", String.class);
+		evaluate("#varargsFunction('a')", "[a]", String.class);
+		evaluate("#varargsFunction('a','b','c')", "[a, b, c]", String.class);
+		// Conversion from int to String
+		evaluate("#varargsFunction(25)", "[25]", String.class);
+		evaluate("#varargsFunction('b',25)", "[b, 25]", String.class);
+		// Strings that contain a comma
+		evaluate("#varargsFunction('a,b')", "[a,b]", String.class);
+		evaluate("#varargsFunction('a', 'x,y', 'd')", "[a, x,y, d]", String.class);
+		// null values
+		evaluate("#varargsFunction(null)", "[null]", String.class);
+		evaluate("#varargsFunction('a',null,'b')", "[a, null, b]", String.class);
 
-		evaluate("#varargsFunctionReverseStringsAndMerge2(1, 'a,b')", "1a,b", String.class);
-		evaluate("#varargsFunctionReverseStringsAndMerge2(1,'a','b','c')", "1cba", String.class);
-		evaluate("#varargsFunctionReverseStringsAndMerge2(1, 'a', 'b,c', 'd')", "1db,ca", String.class);
-		evaluate("#varargsFunctionReverseStringsAndMerge2(2,'a')", "2a", String.class);
-		evaluate("#varargsFunctionReverseStringsAndMerge2(3)", "3", String.class);
-		evaluate("#varargsFunctionReverseStringsAndMerge2(4,'b',25)", "425b", String.class);
-		evaluate("#varargsFunctionReverseStringsAndMerge2(5,25)", "525", String.class);
+		evaluate("#varargsFunction2(9)", "9-[]", String.class);
+		evaluate("#varargsFunction2(9, new String[0])", "9-[]", String.class);
+		evaluate("#varargsFunction2(9,'a')", "9-[a]", String.class);
+		evaluate("#varargsFunction2(9,'a','b','c')", "9-[a, b, c]", String.class);
+		// Conversion from int to String
+		evaluate("#varargsFunction2(9,25)", "9-[25]", String.class);
+		evaluate("#varargsFunction2(9,'b',25)", "9-[b, 25]", String.class);
+		// Strings that contain a comma:
+		evaluate("#varargsFunction2(9, 'a,b')", "9-[a,b]", String.class);
+		evaluate("#varargsFunction2(9, 'a', 'x,y', 'd')", "9-[a, x,y, d]", String.class);
+		// null values
+		evaluate("#varargsFunction2(9,null)", "9-[null]", String.class);
+		evaluate("#varargsFunction2(9,'a',null,'b')", "9-[a, null, b]", String.class);
 	}
 
 	@Test

spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java

@@ -17,11 +17,13 @@
 package org.springframework.expression.spel.testresources;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 
 import org.springframework.util.ObjectUtils;
 
@@ -191,16 +193,17 @@ public String joinThreeStrings(String a, String b, String c) {
 		return a + b + c;
 	}
 
-	public int aVarargsMethod(String... strings) {
-		if (strings == null)
-			return 0;
-		return strings.length;
+	public String aVarargsMethod(String... strings) {
+		return Arrays.toString(strings);
 	}
 
-	public int aVarargsMethod2(int i, String... strings) {
-		if (strings == null)
-			return i;
-		return strings.length + i;
+	public String aVarargsMethod2(int i, String... strings) {
+		return String.valueOf(i) + "-" + Arrays.toString(strings);
+	}
+
+	@SuppressWarnings("unchecked")
+	public String optionalVarargsMethod(Optional<String>... values) {
+		return Arrays.toString(values);
 	}
 
 	public String aVarargsMethod3(String str1, String... strings) {