Fix SpEL vararg method invocation for strings containing commas

Prior to this commit, if a SpEL expression invoked a method or
registered function that declares a String varargs argument, there were
sometimes issues with converting the input arguments into the varargs
array argument. Specifically, if the expression supplied a single
String argument containing a comma for the varargs (such as "a,b"),
SpEL's ReflectionHelper.convertArguments() method incorrectly converted
that single String to an array via the ConversionService, which
indirectly converted that String using the StringToArrayConverter,
which converts a comma-delimited String to an array. Thus, "a,b"
effectively got converted to a two-dimensional array ["a", "b"] instead
of simply ["a,b"].

This commit fixes this bug by avoiding use of the TypeConverter and
ConversionService for single arguments supplied as varargs when the
single argument's type matches the varargs array component type.

Closes gh-27582

Author: sbrannen

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 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.
@@ -38,6 +38,7 @@
  *
  * @author Andy Clement
  * @author Juergen Hoeller
+ * @author Sam Brannen
  * @since 3.0
  */
 public abstract class ReflectionHelper {
@@ -281,25 +282,32 @@ static boolean convertArguments(TypeConverter converter, Object[] arguments, Exe
 				arguments[i] = converter.convertValue(argument, TypeDescriptor.forObject(argument), targetType);
 				conversionOccurred |= (argument != arguments[i]);
 			}
+
 			MethodParameter methodParam = MethodParameter.forExecutable(executable, varargsPosition);
+
+			// If the target is varargs and there is just one more argument, then convert it here.
 			if (varargsPosition == arguments.length - 1) {
-				// If the target is varargs and there is just one more argument
-				// then convert it here
-				TypeDescriptor targetType = new TypeDescriptor(methodParam);
 				Object argument = arguments[varargsPosition];
+				TypeDescriptor targetType = new TypeDescriptor(methodParam);
 				TypeDescriptor sourceType = TypeDescriptor.forObject(argument);
-				arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType);
-				// Three outcomes of that previous line:
-				// 1) the input argument was already compatible (ie. array of valid type) and nothing was done
-				// 2) the input argument was correct type but not in an array so it was made into an array
-				// 3) the input argument was the wrong type and got converted and put into an array
+				// 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())) {
+					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.
 				if (argument != arguments[varargsPosition] &&
 						!isFirstEntryInArray(argument, arguments[varargsPosition])) {
 					conversionOccurred = true; // case 3
 				}
 			}
+			// Otherwise, convert remaining arguments to the varargs element type.
 			else {
-				// Convert remaining arguments to the varargs element type
 				TypeDescriptor targetType = new TypeDescriptor(methodParam).getElementTypeDescriptor();
 				Assert.state(targetType != null, "No element type");
 				for (int i = varargsPosition; i < arguments.length; i++) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 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.
@@ -46,6 +46,7 @@
  *
  * @author Andy Clement
  * @author Phillip Webb
+ * @author Sam Brannen
  */
 public class MethodInvocationTests extends AbstractExpressionTests {
 
@@ -233,26 +234,54 @@ public void testAddingMethodResolvers() {
 
 	@Test
 	public void testVarargsInvocation01() {
-		// Calling 'public int aVarargsMethod(String... strings)'
-		//evaluate("aVarargsMethod('a','b','c')", 3, Integer.class);
-		//evaluate("aVarargsMethod('a')", 1, Integer.class);
+		// 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);
+		evaluate("aVarargsMethod(new String[]{'a','b','c'})", 3, Integer.class);
 	}
 
 	@Test
 	public void testVarargsInvocation02() {
-		// Calling 'public int aVarargsMethod2(int i, String... strings)' - returns int+length_of_strings
+		// 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);
+		evaluate("aVarargsMethod2(8,new String[]{'a','b','c'})", 11, Integer.class);
+	}
+
+	@Test
+	public void testVarargsInvocation03() {
+		// Calling 'public int aVarargsMethod3(String str1, String... strings)' - returns all strings concatenated with "-"
+
+		// No conversion necessary
+		evaluate("aVarargsMethod3('x')", "x", String.class);
+		evaluate("aVarargsMethod3('x', 'a')", "x-a", String.class);
+		evaluate("aVarargsMethod3('x', 'a', 'b', 'c')", "x-a-b-c", String.class);
+
+		// Conversion necessary
+		evaluate("aVarargsMethod3(9)", "9", String.class);
+		evaluate("aVarargsMethod3(8,2,3)", "8-2-3", String.class);
+		evaluate("aVarargsMethod3('2','a',3.0d)", "2-a-3.0", String.class);
+		evaluate("aVarargsMethod3('8',new String[]{'a','b','c'})", "8-a-b-c", String.class);
+
+		// Individual string contains a comma with multiple varargs arguments
+		evaluate("aVarargsMethod3('foo', ',', 'baz')", "foo-,-baz", String.class);
+		evaluate("aVarargsMethod3('foo', 'bar', ',baz')", "foo-bar-,baz", String.class);
+		evaluate("aVarargsMethod3('foo', 'bar,', 'baz')", "foo-bar,-baz", String.class);
+
+		// Individual string contains a comma with single varargs argument.
+		// Reproduces https://github.com/spring-projects/spring-framework/issues/27582
+		evaluate("aVarargsMethod3('foo', ',')", "foo-,", String.class);
+		evaluate("aVarargsMethod3('foo', ',bar')", "foo-,bar", String.class);
+		evaluate("aVarargsMethod3('foo', 'bar,')", "foo-bar,", String.class);
+		evaluate("aVarargsMethod3('foo', 'bar,baz')", "foo-bar,baz", String.class);
 	}
 
 	@Test

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 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.
@@ -28,6 +28,7 @@
 ///CLOVER:OFF
 @SuppressWarnings("unused")
 public class Inventor {
+
 	private String name;
 	public String _name;
 	public String _name_;
@@ -202,8 +203,14 @@ public int aVarargsMethod2(int i, String... strings) {
 		return strings.length + i;
 	}
 
-	public Inventor(String... strings) {
+	public String aVarargsMethod3(String str1, String... strings) {
+		if (ObjectUtils.isEmpty(strings)) {
+			return str1;
+		}
+		return str1 + "-" + String.join("-", strings);
+	}
 
+	public Inventor(String... strings) {
 	}
 
 	public boolean getSomeProperty() {