From d33f66d9b5865d7eb76cfde70e93a25f23c5e4fb Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Wed, 10 Jul 2024 11:51:55 +0200 Subject: [PATCH] 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 --- .../spel/support/ReflectionHelper.java | 6 +-- .../spel/MethodInvocationTests.java | 44 +++++++++++++++++++ .../expression/spel/SpelReproTests.java | 19 +++++--- .../spel/testresources/Inventor.java | 5 +++ 4 files changed, 65 insertions(+), 9 deletions(-) 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 e9b62ec2adb..d7013b57ef0 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-2021 the original author or authors. + * Copyright 2002-2024 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. @@ -298,11 +298,11 @@ public abstract class ReflectionHelper { conversionOccurred = true; } } - // If the argument type is equal to the varargs element type, there is no need to + // If the argument type is assignable to the varargs component 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. - else if (!sourceType.equals(targetType.getElementTypeDescriptor())) { + else if (!sourceType.isAssignableTo(targetType.getElementTypeDescriptor())) { arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType); } // Possible outcomes of the above if-else block: 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 7b59007195e..c1293625d93 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 @@ -294,6 +294,50 @@ public class MethodInvocationTests extends AbstractExpressionTests { evaluate("aVarargsMethod3('foo', 'bar,baz')", "foo-bar,baz", String.class); } + @Test // gh-33013 + void testVarargsWithObjectArrayType() { + // Calling 'public String formatObjectVarargs(String format, Object... args)' -> String.format(format, args) + + // No var-args and no conversion necessary + evaluate("formatObjectVarargs('x')", "x", String.class); + + // No var-args but conversion necessary + evaluate("formatObjectVarargs(9)", "9", String.class); + + // No conversion necessary + evaluate("formatObjectVarargs('x -> %s', '')", "x -> ", String.class); + evaluate("formatObjectVarargs('x -> %s', ' ')", "x -> ", String.class); + evaluate("formatObjectVarargs('x -> %s', 'a')", "x -> a", String.class); + evaluate("formatObjectVarargs('x -> %s %s %s', 'a', 'b', 'c')", "x -> a b c", String.class); + evaluate("formatObjectVarargs('x -> %s', new Object[]{''})", "x -> ", String.class); + evaluate("formatObjectVarargs('x -> %s', new Object[]{' '})", "x -> ", String.class); + evaluate("formatObjectVarargs('x -> %s', new Object[]{'a'})", "x -> a", String.class); + evaluate("formatObjectVarargs('x -> %s %s %s', new Object[]{'a', 'b', 'c'})", "x -> a b c", String.class); + + // The following assertions were cherry-picked from 6.1.x; however, they are expected + // to fail on 6.0.x and 5.3.x, since gh-32704 (Support varargs invocations in SpEL for + // varargs array subtype) was not backported. + // evaluate("formatObjectVarargs('x -> %s', new String[]{''})", "x -> ", String.class); + // evaluate("formatObjectVarargs('x -> %s', new String[]{' '})", "x -> ", String.class); + // evaluate("formatObjectVarargs('x -> %s', new String[]{'a'})", "x -> a", String.class); + // evaluate("formatObjectVarargs('x -> %s %s %s', new String[]{'a', 'b', 'c'})", "x -> a b c", String.class); + + // Conversion necessary + evaluate("formatObjectVarargs('x -> %s %s', 2, 3)", "x -> 2 3", String.class); + evaluate("formatObjectVarargs('x -> %s %s', 'a', 3.0d)", "x -> a 3.0", String.class); + + // Individual string contains a comma with multiple varargs arguments + evaluate("formatObjectVarargs('foo -> %s %s', ',', 'baz')", "foo -> , baz", String.class); + evaluate("formatObjectVarargs('foo -> %s %s', 'bar', ',baz')", "foo -> bar ,baz", String.class); + evaluate("formatObjectVarargs('foo -> %s %s', 'bar,', 'baz')", "foo -> bar, baz", String.class); + + // Individual string contains a comma with single varargs argument. + evaluate("formatObjectVarargs('foo -> %s', ',')", "foo -> ,", String.class); + evaluate("formatObjectVarargs('foo -> %s', ',bar')", "foo -> ,bar", String.class); + evaluate("formatObjectVarargs('foo -> %s', 'bar,')", "foo -> bar,", String.class); + evaluate("formatObjectVarargs('foo -> %s', 'bar,baz')", "foo -> bar,baz", String.class); + } + @Test public void testVarargsOptionalInvocation() { // Calling 'public String optionalVarargsMethod(Optional... values)' diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java index 00d9581f381..f7b9c382bb3 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java @@ -66,6 +66,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatException; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.assertj.core.api.InstanceOfAssertFactories.list; /** * Reproduction tests cornering various reported SpEL issues. @@ -1442,14 +1443,20 @@ class SpelReproTests extends AbstractExpressionTests { assertThat(expression.getValue(new NamedUser())).isEqualTo(NamedUser.class.getName()); } - @Test - @SuppressWarnings("rawtypes") - void SPR12522() { + @Test // gh-17127, SPR-12522 + void arraysAsListWithNoArguments() { + SpelExpressionParser parser = new SpelExpressionParser(); + Expression expression = parser.parseExpression("T(java.util.Arrays).asList()"); + List value = expression.getValue(List.class); + assertThat(value).isEmpty(); + } + + @Test // gh-33013 + void arraysAsListWithSingleEmptyStringArgument() { SpelExpressionParser parser = new SpelExpressionParser(); Expression expression = parser.parseExpression("T(java.util.Arrays).asList('')"); - Object value = expression.getValue(); - assertThat(value).isInstanceOf(List.class); - assertThat(((List) value).isEmpty()).isTrue(); + List value = expression.getValue(List.class); + assertThat(value).asInstanceOf(list(String.class)).containsExactly(""); } @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 282622cf7d2..3979d919e09 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 @@ -213,6 +213,11 @@ public class Inventor { return str1 + "-" + String.join("-", strings); } + public String formatObjectVarargs(String format, Object... args) { + return String.format(format, args); + } + + public Inventor(String... strings) { }