Skip to content

Commit

Permalink
Limit string concatenation in SpEL expressions
Browse files Browse the repository at this point in the history
This commit introduces support for limiting the maximum length of a
string resulting from the concatenation operator (+) in SpEL
expressions.

Closes gh-30332
  • Loading branch information
sbrannen authored and bclozel committed Apr 13, 2023
1 parent 18403cd commit cbbb287
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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");
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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");
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit cbbb287

Please sign in to comment.