From 4d5e7207f20a140258495ad72923c2259e3c800f Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Thu, 16 Mar 2023 17:28:37 +0100 Subject: [PATCH] Improve diagnostics in SpEL for repeated text Attempting to create repeated text in a SpEL expression using the repeat operator can result in errors that are not very helpful to the user. This commit improves the diagnostics in SpEL for the repeat operator by throwing a SpelEvaluationException with a meaningful error message in order to better assist the user. Closes gh-30143 --- .../expression/spel/SpelMessage.java | 8 +++-- .../expression/spel/ast/OpMultiply.java | 29 +++++++++++++++---- .../expression/spel/OperatorTests.java | 21 ++++++++++---- 3 files changed, 46 insertions(+), 12 deletions(-) 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 fe8dd70468..a4cdeafd7b 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -264,7 +264,11 @@ public enum SpelMessage { /** @since 5.3.17 */ MAX_ARRAY_ELEMENTS_THRESHOLD_EXCEEDED(Kind.ERROR, 1075, - "Array declares too many elements, exceeding the threshold of ''{0}''"); + "Array declares too many elements, exceeding the threshold of ''{0}''"), + + /** @since 5.3.26 */ + MAX_REPEATED_TEXT_SIZE_EXCEEDED(Kind.ERROR, 1076, + "Repeated text results in too many characters, exceeding the threshold of ''{0}''"); 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 fbf28317b3..bad9489fb4 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 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. @@ -25,6 +25,8 @@ import org.springframework.expression.Operation; import org.springframework.expression.TypedValue; import org.springframework.expression.spel.CodeFlow; import org.springframework.expression.spel.ExpressionState; +import org.springframework.expression.spel.SpelEvaluationException; +import org.springframework.expression.spel.SpelMessage; import org.springframework.util.Assert; import org.springframework.util.NumberUtils; @@ -52,6 +54,13 @@ import org.springframework.util.NumberUtils; */ public class OpMultiply extends Operator { + /** + * Maximum number of characters permitted in repeated text. + * @since 5.3.26 + */ + private static final int MAX_REPEATED_TEXT_SIZE = 256; + + public OpMultiply(int startPos, int endPos, SpelNodeImpl... operands) { super("*", startPos, endPos, operands); } @@ -109,10 +118,13 @@ public class OpMultiply extends Operator { } if (leftOperand instanceof String && rightOperand instanceof Integer) { - int repeats = (Integer) rightOperand; - StringBuilder result = new StringBuilder(); - for (int i = 0; i < repeats; i++) { - result.append(leftOperand); + String text = (String) leftOperand; + int count = (Integer) rightOperand; + int requestedSize = text.length() * count; + checkRepeatedTextSize(requestedSize); + StringBuilder result = new StringBuilder(requestedSize); + for (int i = 0; i < count; i++) { + result.append(text); } return new TypedValue(result.toString()); } @@ -120,6 +132,13 @@ public class OpMultiply extends Operator { return state.operate(Operation.MULTIPLY, leftOperand, rightOperand); } + private void checkRepeatedTextSize(int requestedSize) { + if (requestedSize > MAX_REPEATED_TEXT_SIZE) { + throw new SpelEvaluationException(getStartPosition(), + SpelMessage.MAX_REPEATED_TEXT_SIZE_EXCEEDED, MAX_REPEATED_TEXT_SIZE); + } + } + @Override public boolean isCompilable() { if (!getLeftOperand().isCompilable()) { 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 c5e53660f2..bd0a8c2f99 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 @@ -21,10 +21,12 @@ import java.math.BigInteger; import org.junit.jupiter.api.Test; +import org.springframework.expression.Expression; import org.springframework.expression.spel.ast.Operator; import org.springframework.expression.spel.standard.SpelExpression; import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.expression.spel.SpelMessage.MAX_REPEATED_TEXT_SIZE_EXCEEDED; /** * Tests the evaluation of expressions using various operators. @@ -32,6 +34,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Andy Clement * @author Juergen Hoeller * @author Giovanni Dall'Oglio Risso + * @author Sam Brannen */ class OperatorTests extends AbstractExpressionTests { @@ -324,11 +327,6 @@ class OperatorTests extends AbstractExpressionTests { evaluate("3.5", 3.5d, Double.class); } - @Test - void multiplyStringInt() { - evaluate("'a' * 5", "aaaaa", String.class); - } - @Test void multiplyDoubleDoubleGivesDouble() { evaluate("3.0d * 5.0d", 15.0d, Double.class); @@ -576,6 +574,19 @@ class OperatorTests extends AbstractExpressionTests { evaluate("'abc' != 'def'", true, Boolean.class); } + @Test + void stringRepeat() { + evaluate("'abc' * 0", "", String.class); + evaluate("'abc' * 1", "abc", String.class); + evaluate("'abc' * 2", "abcabc", String.class); + + Expression expr = parser.parseExpression("'a' * 256"); + assertThat(expr.getValue(context, String.class)).hasSize(256); + + // 4 is the position of the '*' (repeat operator) + evaluateAndCheckError("'a' * 257", String.class, MAX_REPEATED_TEXT_SIZE_EXCEEDED, 4); + } + @Test void longs() { evaluate("3L == 4L", false, Boolean.class);