Skip to content

Commit

Permalink
Improve diagnostics for negative repeated text count in SpEL
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sbrannen committed Sep 29, 2023
1 parent 8e83f93 commit 35f458f
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ else if (CodeFlow.isIntegerForNumericOp(leftNumber) || CodeFlow.isIntegerForNume
}

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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
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.
Expand Down Expand Up @@ -587,6 +588,13 @@ void stringRepeat() {
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);
Expand Down

0 comments on commit 35f458f

Please sign in to comment.