diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java index 65e3cff83a..821c0b246a 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java +++ b/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 @@ import org.springframework.util.MethodInvoker; * * @author Andy Clement * @author Juergen Hoeller + * @author Sam Brannen * @since 3.0 */ public abstract class ReflectionHelper { @@ -281,25 +282,32 @@ public abstract class ReflectionHelper { 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++) { diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java index 0a025acf72..b2cde1f10f 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java +++ b/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 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; * * @author Andy Clement * @author Phillip Webb + * @author Sam Brannen */ public class MethodInvocationTests extends AbstractExpressionTests { @@ -233,26 +234,54 @@ public class MethodInvocationTests extends AbstractExpressionTests { @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 diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java b/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java index bdf6d79c1d..34960d982f 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java +++ b/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 @@ import org.springframework.util.ObjectUtils; ///CLOVER:OFF @SuppressWarnings("unused") public class Inventor { + private String name; public String _name; public String _name_; @@ -202,8 +203,14 @@ public class Inventor { 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() {