From 35f458fa5fef5ea4e91f825d26ae06e9e58cbfdb Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Fri, 29 Sep 2023 17:45:15 +0200 Subject: [PATCH] Improve diagnostics for negative repeated text count in SpEL Due to the changes in gh-31341, if the repeat count in a SpEL expression (using the repeat operator '*') is negative, we throw a SpelEvaluationException with the MAX_REPEATED_TEXT_SIZE_EXCEEDED message which is incorrect and misleading. Prior to gh-31341, a negative repeat count resulted in an IllegalArgumentException being thrown by String#repeat(), which was acceptable in terms of diagnostics, but that did not make it immediately clear to the user what the underlying cause was. In light of the above, this commit improves diagnostics for a negative repeated text count in SpEL expressions by throwing a SpelEvaluationException with a new NEGATIVE_REPEATED_TEXT_COUNT error message. Closes gh-31342 --- .../org/springframework/expression/spel/SpelMessage.java | 6 +++++- .../springframework/expression/spel/ast/OpMultiply.java | 4 ++++ .../springframework/expression/spel/OperatorTests.java | 8 ++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java b/spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java index b181cda36d8..d9735071d1f 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/SpelMessage.java @@ -284,7 +284,11 @@ public enum SpelMessage { /** @since 5.2.24 */ VARIABLE_ASSIGNMENT_NOT_SUPPORTED(Kind.ERROR, 1080, - "Assignment to variable ''{0}'' is not supported"); + "Assignment to variable ''{0}'' is not supported"), + + /** @since 6.0.13 */ + NEGATIVE_REPEATED_TEXT_COUNT(Kind.ERROR, 1081, + "Repeat count ''{0}'' must not be negative"); private final Kind kind; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMultiply.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMultiply.java index 559a55817f9..fce5a8951a6 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMultiply.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMultiply.java @@ -123,6 +123,10 @@ public class OpMultiply extends Operator { } private void checkRepeatedTextSize(String text, int count) { + if (count < 0) { + throw new SpelEvaluationException(getStartPosition(), + SpelMessage.NEGATIVE_REPEATED_TEXT_COUNT, count); + } int result = text.length() * count; if (result < 0 || result > MAX_REPEATED_TEXT_SIZE) { throw new SpelEvaluationException(getStartPosition(), diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/OperatorTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/OperatorTests.java index cbbb153c9b0..cb66885d8c5 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/OperatorTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/OperatorTests.java @@ -28,6 +28,7 @@ import org.springframework.expression.spel.standard.SpelExpression; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.expression.spel.SpelMessage.MAX_CONCATENATED_STRING_LENGTH_EXCEEDED; import static org.springframework.expression.spel.SpelMessage.MAX_REPEATED_TEXT_SIZE_EXCEEDED; +import static org.springframework.expression.spel.SpelMessage.NEGATIVE_REPEATED_TEXT_COUNT; /** * Tests the evaluation of expressions using various operators. @@ -587,6 +588,13 @@ class OperatorTests extends AbstractExpressionTests { evaluateAndCheckError("'ab' * " + repeatCount, String.class, MAX_REPEATED_TEXT_SIZE_EXCEEDED, 5); } + @Test + void stringRepeatWithNegativeRepeatCount() { + // 4 is the position of the '*' (repeat operator) + // -1 is the negative repeat count + evaluateAndCheckError("'a' * -1", String.class, NEGATIVE_REPEATED_TEXT_COUNT, 4, -1); + } + @Test void stringConcatenation() { evaluate("'' + ''", "", String.class);