Skip to content

Commit d33f66d

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-33189
1 parent 8da2963 commit d33f66d

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
@@ -66,6 +66,7 @@
6666
import static org.assertj.core.api.Assertions.assertThatException;
6767
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
6868
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
69+
import static org.assertj.core.api.InstanceOfAssertFactories.list;
6970

7071
/**
7172
* Reproduction tests cornering various reported SpEL issues.
@@ -1442,14 +1443,20 @@ void SPR12502() {
14421443
assertThat(expression.getValue(new NamedUser())).isEqualTo(NamedUser.class.getName());
14431444
}
14441445

1445-
@Test
1446-
@SuppressWarnings("rawtypes")
1447-
void SPR12522() {
1446+
@Test // gh-17127, SPR-12522
1447+
void arraysAsListWithNoArguments() {
1448+
SpelExpressionParser parser = new SpelExpressionParser();
1449+
Expression expression = parser.parseExpression("T(java.util.Arrays).asList()");
1450+
List<?> value = expression.getValue(List.class);
1451+
assertThat(value).isEmpty();
1452+
}
1453+
1454+
@Test // gh-33013
1455+
void arraysAsListWithSingleEmptyStringArgument() {
14481456
SpelExpressionParser parser = new SpelExpressionParser();
14491457
Expression expression = parser.parseExpression("T(java.util.Arrays).asList('')");
1450-
Object value = expression.getValue();
1451-
assertThat(value).isInstanceOf(List.class);
1452-
assertThat(((List) value).isEmpty()).isTrue();
1458+
List<?> value = expression.getValue(List.class);
1459+
assertThat(value).asInstanceOf(list(String.class)).containsExactly("");
14531460
}
14541461

14551462
@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)