From 595053fa15312c769e862f3d62d9d341057014ca Mon Sep 17 00:00:00 2001 From: Cesar Hernandez Date: Thu, 20 Apr 2023 20:25:49 -0600 Subject: [PATCH 1/3] Change max regex length in SpEL expressions to 1000 This commit changes the max regex length in SpEL expressions from 1024 to 1000 in order to consistently use round numbers for recently introduced limits. See gh-30265 --- .../expression/spel/ast/OperatorMatches.java | 2 +- .../expression/spel/EvaluationTests.java | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorMatches.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorMatches.java index a6cb5fa89314..7e332352feae 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorMatches.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorMatches.java @@ -46,7 +46,7 @@ public class OperatorMatches extends Operator { * Maximum number of characters permitted in a regular expression. * @since 5.3.26 */ - private static final int MAX_REGEX_LENGTH = 256; + private static final int MAX_REGEX_LENGTH = 1000; private final ConcurrentMap patternCache; diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java index 37f9cc29749e..eeee9badb41b 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java @@ -212,18 +212,24 @@ public void testMatchesWithPatternAccessThreshold() { @Test public void matchesWithPatternLengthThreshold() { - String pattern = "(0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789" + - "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789" + - "01234567890123456789012345678901234567890123456789|abc)"; - assertEquals(256, pattern.length()); - Expression expr = parser.parseExpression("'abc' matches '" + pattern + "'"); + String pattern = String.format("^(%s|X)", repeat("12345", 199)); + assertEquals(1000, pattern.length()); + Expression expr = parser.parseExpression("'X' matches '" + pattern + "'"); assertTrue(expr.getValue(context, Boolean.class)); pattern += "?"; - assertEquals(257, pattern.length()); + assertEquals(1001, pattern.length()); evaluateAndCheckError("'abc' matches '" + pattern + "'", Boolean.class, SpelMessage.MAX_REGEX_LENGTH_EXCEEDED); } + private String repeat(String str, int count) { + String result = ""; + for (int i = 0; i < count; i++) { + result += str; + } + return result; + } + // mixing operators @Test public void testMixingOperators01() { From 083c56e97f94002c312e8b0578e0094bfe0db7d2 Mon Sep 17 00:00:00 2001 From: Cesar Hernandez Date: Thu, 20 Apr 2023 20:45:52 -0600 Subject: [PATCH 2/3] Limit string concatenation in SpEL expressions This commit introduces support for limiting the maximum length of a string resulting from the concatenation operator (+) in SpEL expressions. Closes gh-30332 --- .../expression/spel/SpelMessage.java | 6 +- .../expression/spel/ast/OpPlus.java | 42 +++++++++-- .../expression/spel/OperatorTests.java | 73 ++++++++++++++++--- 3 files changed, 105 insertions(+), 16 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 c00bea12a0ee..4cfc0c2f45a2 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 @@ -268,7 +268,11 @@ public enum SpelMessage { /** @since 5.3.26 */ MAX_REGEX_LENGTH_EXCEEDED(Kind.ERROR, 1077, - "Regular expression contains too many characters, exceeding the threshold of ''{0}''"); + "Regular expression contains too many characters, exceeding the threshold of ''{0}''"), + + /** @since 5.2.24 */ + MAX_CONCATENATED_STRING_LENGTH_EXCEEDED(Kind.ERROR, 1078, + "Concatenated string is too long, exceeding the threshold of ''{0}'' characters"); private final Kind kind; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpPlus.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpPlus.java index f60b1cd715dd..cb323d6f8466 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpPlus.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpPlus.java @@ -27,6 +27,9 @@ 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.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.NumberUtils; @@ -49,6 +52,12 @@ */ public class OpPlus extends Operator { + /** + * Maximum number of characters permitted in a concatenated string. + * @since 5.2.24 + */ + private static final int MAX_CONCATENATED_STRING_LENGTH = 100000; + public OpPlus(int pos, SpelNodeImpl... operands) { super("+", pos, operands); Assert.notEmpty(operands, "Operands must not be empty"); @@ -123,22 +132,45 @@ else if (CodeFlow.isIntegerForNumericOp(leftNumber) || CodeFlow.isIntegerForNume if (leftOperand instanceof String && rightOperand instanceof String) { this.exitTypeDescriptor = "Ljava/lang/String"; - return new TypedValue((String) leftOperand + rightOperand); + String leftString = (String) leftOperand; + String rightString = (String) rightOperand; + checkStringLength(leftString); + checkStringLength(rightString); + return concatenate(leftString, rightString); } if (leftOperand instanceof String) { - return new TypedValue( - leftOperand + (rightOperand == null ? "null" : convertTypedValueToString(operandTwoValue, state))); + String leftString = (String) leftOperand; + checkStringLength(leftString); + String rightString = (rightOperand == null ? "null" : convertTypedValueToString(operandTwoValue, state)); + checkStringLength(rightString); + return concatenate(leftString, rightString); } if (rightOperand instanceof String) { - return new TypedValue( - (leftOperand == null ? "null" : convertTypedValueToString(operandOneValue, state)) + rightOperand); + String rightString = (String) rightOperand; + checkStringLength(rightString); + String leftString = (leftOperand == null ? "null" : convertTypedValueToString(operandOneValue, state)); + checkStringLength(leftString); + return concatenate(leftString, rightString); } return state.operate(Operation.ADD, leftOperand, rightOperand); } + private void checkStringLength(String string) { + if (string.length() > MAX_CONCATENATED_STRING_LENGTH) { + throw new SpelEvaluationException(getStartPosition(), + SpelMessage.MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, MAX_CONCATENATED_STRING_LENGTH); + } + } + + private TypedValue concatenate(String leftString, String rightString) { + String result = leftString + rightString; + checkStringLength(result); + return new TypedValue(result); + } + @Override public String toStringAST() { if (this.children.length < 2) { // unary plus 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 28d31b39aa8e..f2481aefdcfa 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 @@ -27,6 +27,7 @@ import static org.junit.Assert.*; import static org.springframework.expression.spel.SpelMessage.MAX_REPEATED_TEXT_SIZE_EXCEEDED; +import static org.springframework.expression.spel.SpelMessage.MAX_CONCATENATED_STRING_LENGTH_EXCEEDED; /** * Tests the evaluation of expressions using relational operators. @@ -389,11 +390,7 @@ public void testPlus() throws Exception { evaluate("3.0f + 5.0f", 8.0f, Float.class); evaluate("3.0d + 5.0d", 8.0d, Double.class); evaluate("3 + new java.math.BigDecimal('5')", new BigDecimal("8"), BigDecimal.class); - - evaluate("'ab' + 2", "ab2", String.class); - evaluate("2 + 'a'", "2a", String.class); - evaluate("'ab' + null", "abnull", String.class); - evaluate("null + 'ab'", "nullab", String.class); + evaluate("5 + new Integer('37')", 42, Integer.class); // AST: SpelExpression expr = (SpelExpression)parser.parseExpression("+3"); @@ -402,11 +399,11 @@ public void testPlus() throws Exception { assertEquals("(2 + 3)",expr.toStringAST()); // use as a unary operator - evaluate("+5d",5d,Double.class); - evaluate("+5L",5L,Long.class); - evaluate("+5",5,Integer.class); - evaluate("+new java.math.BigDecimal('5')", new BigDecimal("5"),BigDecimal.class); - evaluateAndCheckError("+'abc'",SpelMessage.OPERATOR_NOT_SUPPORTED_BETWEEN_TYPES); + evaluate("+5d", 5d, Double.class); + evaluate("+5L", 5L, Long.class); + evaluate("+5", 5, Integer.class); + evaluate("+new java.math.BigDecimal('5')", new BigDecimal("5"), BigDecimal.class); + evaluateAndCheckError("+'abc'", SpelMessage.OPERATOR_NOT_SUPPORTED_BETWEEN_TYPES); // string concatenation evaluate("'abc'+'def'","abcdef",String.class); @@ -585,6 +582,62 @@ public void stringRepeat() { evaluateAndCheckError("'a' * 257", String.class, MAX_REPEATED_TEXT_SIZE_EXCEEDED, 4); } + @Test + public void stringConcatenation() { + evaluate("'' + ''", "", String.class); + evaluate("'' + null", "null", String.class); + evaluate("null + ''", "null", String.class); + evaluate("'ab' + null", "abnull", String.class); + evaluate("null + 'ab'", "nullab", String.class); + evaluate("'ab' + 2", "ab2", String.class); + evaluate("2 + 'ab'", "2ab", String.class); + evaluate("'abc' + 'def'", "abcdef", String.class); + + // Text is big but not too big + final int maxSize = 100_000; + context.setVariable("text1", createString(maxSize)); + Expression expr = parser.parseExpression("#text1 + ''"); + //assertThat(expr.getValue(context, String.class)).hasSize(maxSize); + assertEquals(maxSize, expr.getValue(context, String.class).length()); + + expr = parser.parseExpression("'' + #text1"); + //assertThat(expr.getValue(context, String.class)).hasSize(maxSize); + assertEquals(maxSize, expr.getValue(context, String.class).length()); + + context.setVariable("text1", createString(maxSize / 2)); + expr = parser.parseExpression("#text1 + #text1"); + //assertThat(expr.getValue(context, String.class)).hasSize(maxSize); + assertEquals(maxSize, expr.getValue(context, String.class).length()); + + // Text is too big + context.setVariable("text1", createString(maxSize + 1)); + evaluateAndCheckError("#text1 + ''", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 7); + evaluateAndCheckError("#text1 + true", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 7); + evaluateAndCheckError("'' + #text1", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 3); + evaluateAndCheckError("true + #text1", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 5); + + context.setVariable("text1", createString(maxSize / 2)); + context.setVariable("text2", createString((maxSize / 2) + 1)); + evaluateAndCheckError("#text1 + #text2", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 7); + evaluateAndCheckError("#text1 + #text2 + true", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 7); + evaluateAndCheckError("#text1 + true + #text2", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 14); + evaluateAndCheckError("true + #text1 + #text2", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 14); + + evaluateAndCheckError("#text2 + #text1", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 7); + evaluateAndCheckError("#text2 + #text1 + true", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 7); + evaluateAndCheckError("#text2 + true + #text1", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 14); + evaluateAndCheckError("true + #text2 + #text1", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 14); + + context.setVariable("text1", createString((maxSize / 3) + 1)); + evaluateAndCheckError("#text1 + #text1 + #text1", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 16); + evaluateAndCheckError("(#text1 + #text1) + #text1", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 18); + evaluateAndCheckError("#text1 + (#text1 + #text1)", String.class, MAX_CONCATENATED_STRING_LENGTH_EXCEEDED, 7); + } + + private static String createString(int size) { + return new String(new char[size]); + } + @Test public void testLongs() { evaluate("3L == 4L", false, Boolean.class); From 820b701da5ad4440bb16de11e7345865af686d98 Mon Sep 17 00:00:00 2001 From: Cesar Hernandez Date: Thu, 20 Apr 2023 20:59:23 -0600 Subject: [PATCH 3/3] Limit SpEL expression length This commit enforces a limit of the maximum size of a single SpEL expression. Closes gh-30330 --- .../expression/spel/SpelMessage.java | 14 ++++++--- .../InternalSpelExpressionParser.java | 16 ++++++++++ .../expression/spel/EvaluationTests.java | 31 ++++++++++++++----- 3 files changed, 48 insertions(+), 13 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 4cfc0c2f45a2..ba6a7284fc41 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 @@ -262,17 +262,21 @@ public enum SpelMessage { MAX_ARRAY_ELEMENTS_THRESHOLD_EXCEEDED(Kind.ERROR, 1075, "Array declares too many elements, exceeding the threshold of ''{0}''"), - /** @since 5.3.26 */ + /** @since 5.2.23 */ MAX_REPEATED_TEXT_SIZE_EXCEEDED(Kind.ERROR, 1076, - "Repeated text results in too many characters, exceeding the threshold of ''{0}''"), + "Repeated text is too long, exceeding the threshold of ''{0}'' characters"), - /** @since 5.3.26 */ + /** @since 5.2.23 */ MAX_REGEX_LENGTH_EXCEEDED(Kind.ERROR, 1077, - "Regular expression contains too many characters, exceeding the threshold of ''{0}''"), + "Regular expression is too long, exceeding the threshold of ''{0}'' characters"), /** @since 5.2.24 */ MAX_CONCATENATED_STRING_LENGTH_EXCEEDED(Kind.ERROR, 1078, - "Concatenated string is too long, exceeding the threshold of ''{0}'' characters"); + "Concatenated string is too long, exceeding the threshold of ''{0}'' characters"), + + /** @since 5.2.24 */ + MAX_EXPRESSION_LENGTH_EXCEEDED(Kind.ERROR, 1079, + "SpEL expression is too long, exceeding the threshold of ''{0}'' characters"); private final Kind kind; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java b/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java index 07c8ccb01fb8..e73c73c64b71 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/standard/InternalSpelExpressionParser.java @@ -28,6 +28,7 @@ import org.springframework.expression.ParserContext; import org.springframework.expression.common.TemplateAwareExpressionParser; import org.springframework.expression.spel.InternalParseException; +import org.springframework.expression.spel.SpelEvaluationException; import org.springframework.expression.spel.SpelMessage; import org.springframework.expression.spel.SpelParseException; import org.springframework.expression.spel.SpelParserConfiguration; @@ -90,6 +91,12 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser { private static final Pattern VALID_QUALIFIED_ID_PATTERN = Pattern.compile("[\\p{L}\\p{N}_$]+"); + /** + * Maximum length permitted for a SpEL expression. + * @since 5.2.24 + */ + private static final int MAX_EXPRESSION_LENGTH = 10000; + private final SpelParserConfiguration configuration; @@ -123,6 +130,9 @@ public InternalSpelExpressionParser(SpelParserConfiguration configuration) { @Override protected SpelExpression doParseExpression(String expressionString, ParserContext context) throws ParseException { + + checkExpressionLength(expressionString); + try { this.expressionString = expressionString; Tokenizer tokenizer = new Tokenizer(expressionString); @@ -142,6 +152,12 @@ protected SpelExpression doParseExpression(String expressionString, ParserContex } } + private void checkExpressionLength(String string) { + if (string.length() > MAX_EXPRESSION_LENGTH) { + throw new SpelEvaluationException(SpelMessage.MAX_EXPRESSION_LENGTH_EXCEEDED, MAX_EXPRESSION_LENGTH); + } + } + // expression // : logicalOrExpression // ( (ASSIGN^ logicalOrExpression) diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java index eeee9badb41b..72fd4f3e2da2 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java @@ -57,6 +57,20 @@ */ public class EvaluationTests extends AbstractExpressionTests { + @Test + public void expressionLength() { + String expression = String.format("'X' + '%s'", repeat(" ", 9992)); + assertEquals(10000, expression.length()); + Expression expr = parser.parseExpression(expression); + String result = expr.getValue(context, String.class); + assertEquals(9993, result.length()); + assertEquals("X", result.trim()); + + expression = String.format("'X' + '%s'", repeat(" ", 9993)); + assertEquals(10001, expression.length()); + evaluateAndCheckError(expression, String.class, SpelMessage.MAX_EXPRESSION_LENGTH_EXCEEDED); + } + @Test public void testCreateListsOnAttemptToIndexNull01() throws EvaluationException, ParseException { ExpressionParser parser = new SpelExpressionParser(new SpelParserConfiguration(true, true)); @@ -222,14 +236,6 @@ public void matchesWithPatternLengthThreshold() { evaluateAndCheckError("'abc' matches '" + pattern + "'", Boolean.class, SpelMessage.MAX_REGEX_LENGTH_EXCEEDED); } - private String repeat(String str, int count) { - String result = ""; - for (int i = 0; i < count; i++) { - result += str; - } - return result; - } - // mixing operators @Test public void testMixingOperators01() { @@ -1444,6 +1450,15 @@ public List filter(List methods) { } + private static String repeat(String str, int count) { + String result = ""; + for (int i = 0; i < count; i++) { + result += str; + } + return result; + } + + @SuppressWarnings("rawtypes") static class TestClass {