Skip to content

Commit dffb6c5

Browse files
committed
Support single String argument for varargs invocations in SpEL
Prior to this commit, the Spring Expression Language (SpEL) incorrectly split single String arguments by comma for Object... varargs method and constructor invocations. This commit addresses this by checking if the single argument type is already "assignable" to the varargs component type instead of "equal" to the varargs component type. See gh-33013 Closes gh-33188 (cherry picked from commit d33f66d)
1 parent 286f5f2 commit dffb6c5

File tree

4 files changed

+65
-9
lines changed

4 files changed

+65
-9
lines changed

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -298,11 +298,11 @@ static boolean convertArguments(TypeConverter converter, Object[] arguments, Exe
298298
conversionOccurred = true;
299299
}
300300
}
301-
// If the argument type is equal to the varargs element type, there is no need to
301+
// If the argument type is assignable to the varargs component type, there is no need to
302302
// convert it or wrap it in an array. For example, using StringToArrayConverter to
303303
// convert a String containing a comma would result in the String being split and
304304
// repackaged in an array when it should be used as-is.
305-
else if (!sourceType.equals(targetType.getElementTypeDescriptor())) {
305+
else if (!sourceType.isAssignableTo(targetType.getElementTypeDescriptor())) {
306306
arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType);
307307
}
308308
// Possible outcomes of the above if-else block:

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

+44
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,50 @@ public void testVarargsInvocation03() {
294294
evaluate("aVarargsMethod3('foo', 'bar,baz')", "foo-bar,baz", String.class);
295295
}
296296

297+
@Test // gh-33013
298+
void testVarargsWithObjectArrayType() {
299+
// Calling 'public String formatObjectVarargs(String format, Object... args)' -> String.format(format, args)
300+
301+
// No var-args and no conversion necessary
302+
evaluate("formatObjectVarargs('x')", "x", String.class);
303+
304+
// No var-args but conversion necessary
305+
evaluate("formatObjectVarargs(9)", "9", String.class);
306+
307+
// No conversion necessary
308+
evaluate("formatObjectVarargs('x -> %s', '')", "x -> ", String.class);
309+
evaluate("formatObjectVarargs('x -> %s', ' ')", "x -> ", String.class);
310+
evaluate("formatObjectVarargs('x -> %s', 'a')", "x -> a", String.class);
311+
evaluate("formatObjectVarargs('x -> %s %s %s', 'a', 'b', 'c')", "x -> a b c", String.class);
312+
evaluate("formatObjectVarargs('x -> %s', new Object[]{''})", "x -> ", String.class);
313+
evaluate("formatObjectVarargs('x -> %s', new Object[]{' '})", "x -> ", String.class);
314+
evaluate("formatObjectVarargs('x -> %s', new Object[]{'a'})", "x -> a", String.class);
315+
evaluate("formatObjectVarargs('x -> %s %s %s', new Object[]{'a', 'b', 'c'})", "x -> a b c", String.class);
316+
317+
// The following assertions were cherry-picked from 6.1.x; however, they are expected
318+
// to fail on 6.0.x and 5.3.x, since gh-32704 (Support varargs invocations in SpEL for
319+
// varargs array subtype) was not backported.
320+
// evaluate("formatObjectVarargs('x -> %s', new String[]{''})", "x -> ", String.class);
321+
// evaluate("formatObjectVarargs('x -> %s', new String[]{' '})", "x -> ", String.class);
322+
// evaluate("formatObjectVarargs('x -> %s', new String[]{'a'})", "x -> a", String.class);
323+
// evaluate("formatObjectVarargs('x -> %s %s %s', new String[]{'a', 'b', 'c'})", "x -> a b c", String.class);
324+
325+
// Conversion necessary
326+
evaluate("formatObjectVarargs('x -> %s %s', 2, 3)", "x -> 2 3", String.class);
327+
evaluate("formatObjectVarargs('x -> %s %s', 'a', 3.0d)", "x -> a 3.0", String.class);
328+
329+
// Individual string contains a comma with multiple varargs arguments
330+
evaluate("formatObjectVarargs('foo -> %s %s', ',', 'baz')", "foo -> , baz", String.class);
331+
evaluate("formatObjectVarargs('foo -> %s %s', 'bar', ',baz')", "foo -> bar ,baz", String.class);
332+
evaluate("formatObjectVarargs('foo -> %s %s', 'bar,', 'baz')", "foo -> bar, baz", String.class);
333+
334+
// Individual string contains a comma with single varargs argument.
335+
evaluate("formatObjectVarargs('foo -> %s', ',')", "foo -> ,", String.class);
336+
evaluate("formatObjectVarargs('foo -> %s', ',bar')", "foo -> ,bar", String.class);
337+
evaluate("formatObjectVarargs('foo -> %s', 'bar,')", "foo -> bar,", String.class);
338+
evaluate("formatObjectVarargs('foo -> %s', 'bar,baz')", "foo -> bar,baz", String.class);
339+
}
340+
297341
@Test
298342
public void testVarargsOptionalInvocation() {
299343
// Calling 'public String optionalVarargsMethod(Optional<String>... values)'

Diff for: spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java

+13-6
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import static org.assertj.core.api.Assertions.assertThatException;
6666
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
6767
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
68+
import static org.assertj.core.api.InstanceOfAssertFactories.list;
6869

6970
/**
7071
* Reproduction tests cornering various reported SpEL issues.
@@ -1460,14 +1461,20 @@ void SPR12502() {
14601461
assertThat(expression.getValue(new NamedUser())).isEqualTo(NamedUser.class.getName());
14611462
}
14621463

1463-
@Test
1464-
@SuppressWarnings("rawtypes")
1465-
void SPR12522() {
1464+
@Test // gh-17127, SPR-12522
1465+
void arraysAsListWithNoArguments() {
1466+
SpelExpressionParser parser = new SpelExpressionParser();
1467+
Expression expression = parser.parseExpression("T(java.util.Arrays).asList()");
1468+
List<?> value = expression.getValue(List.class);
1469+
assertThat(value).isEmpty();
1470+
}
1471+
1472+
@Test // gh-33013
1473+
void arraysAsListWithSingleEmptyStringArgument() {
14661474
SpelExpressionParser parser = new SpelExpressionParser();
14671475
Expression expression = parser.parseExpression("T(java.util.Arrays).asList('')");
1468-
Object value = expression.getValue();
1469-
assertThat(value instanceof List).isTrue();
1470-
assertThat(((List) value).isEmpty()).isTrue();
1476+
List<?> value = expression.getValue(List.class);
1477+
assertThat(value).asInstanceOf(list(String.class)).containsExactly("");
14711478
}
14721479

14731480
@Test

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

+5
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,11 @@ public String aVarargsMethod3(String str1, String... strings) {
213213
return str1 + "-" + String.join("-", strings);
214214
}
215215

216+
public String formatObjectVarargs(String format, Object... args) {
217+
return String.format(format, args);
218+
}
219+
220+
216221
public Inventor(String... strings) {
217222
}
218223

0 commit comments

Comments
 (0)