From cbbb2871b3a4092c623a4a87c4dd24f54aa24225 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Fri, 7 Apr 2023 17:02:09 +0200 Subject: [PATCH] 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 | 45 ++++++++++-- .../expression/spel/OperatorTests.java | 70 ++++++++++++++++--- 3 files changed, 104 insertions(+), 17 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 f1fd0f67f715..3ee35997fd1b 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.2.23 */ 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 d128f56b2deb..365aa8399b29 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 @@ -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. @@ -27,6 +27,8 @@ 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; @@ -46,10 +48,18 @@ * @author Juergen Hoeller * @author Ivo Smid * @author Giovanni Dall'Oglio Risso + * @author Sam Brannen * @since 3.0 */ 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 = 100_000; + + public OpPlus(int startPos, int endPos, SpelNodeImpl... operands) { super("+", startPos, endPos, operands); Assert.notEmpty(operands, "Operands must not be empty"); @@ -123,22 +133,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 26a219ece332..83904eb1a68d 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 @@ -26,6 +26,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; /** @@ -391,11 +392,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"); @@ -404,11 +401,11 @@ public void testPlus() throws Exception { assertThat(expr.toStringAST()).isEqualTo("(2 + 3)"); // 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); @@ -587,6 +584,59 @@ void stringRepeat() { evaluateAndCheckError("'a' * 257", String.class, MAX_REPEATED_TEXT_SIZE_EXCEEDED, 4); } + @Test + 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); + + expr = parser.parseExpression("'' + #text1"); + assertThat(expr.getValue(context, String.class)).hasSize(maxSize); + + context.setVariable("text1", createString(maxSize / 2)); + expr = parser.parseExpression("#text1 + #text1"); + assertThat(expr.getValue(context, String.class)).hasSize(maxSize); + + // 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);