From de113f1d11e6b995c6beaf70a3d3ce116d9915ec Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 25 Apr 2023 13:16:10 +0200 Subject: [PATCH] Reject null and empty SpEL expressions Prior to gh-30325, supplying a null reference for a SpEL expression was effectively equivalent to supplying the String "null" as the expression. Consequently, evaluation of a null reference expression always evaluated to a null reference. However, that was accidental rather than by design. Due to the introduction of the checkExpressionLength(String) method in InternalSpelExpressionParser (in conjunction with gh-30325), an attempt to evaluate a null reference as a SpEL expression now results in a NullPointerException. To address both of these issues, TemplateAwareExpressionParser.parseExpression() and SpelExpressionParser.parseRaw() now reject null and empty SpEL expressions. Closes gh-30371 --- .../common/TemplateAwareExpressionParser.java | 6 ++++- .../spel/standard/SpelExpressionParser.java | 4 ++- .../spel/TemplateExpressionParsingTests.java | 7 +++++ .../spel/standard/SpelParserTests.java | 27 +++++++++++++++++++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/common/TemplateAwareExpressionParser.java b/spring-expression/src/main/java/org/springframework/expression/common/TemplateAwareExpressionParser.java index 58bae54976bd..0f5ccbddbd27 100644 --- a/spring-expression/src/main/java/org/springframework/expression/common/TemplateAwareExpressionParser.java +++ b/spring-expression/src/main/java/org/springframework/expression/common/TemplateAwareExpressionParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 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. @@ -26,6 +26,7 @@ import org.springframework.expression.ParseException; import org.springframework.expression.ParserContext; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * An expression parser that understands templates. It can be subclassed by expression @@ -34,6 +35,7 @@ * @author Keith Donald * @author Juergen Hoeller * @author Andy Clement + * @author Sam Brannen * @since 3.0 */ public abstract class TemplateAwareExpressionParser implements ExpressionParser { @@ -46,9 +48,11 @@ public Expression parseExpression(String expressionString) throws ParseException @Override public Expression parseExpression(String expressionString, @Nullable ParserContext context) throws ParseException { if (context != null && context.isTemplate()) { + Assert.notNull(expressionString, "'expressionString' must not be null"); return parseTemplate(expressionString, context); } else { + Assert.hasText(expressionString, "'expressionString' must not be null or blank"); return doParseExpression(expressionString, context); } } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpressionParser.java b/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpressionParser.java index a8702b41bd65..ee64ba4da2e0 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpressionParser.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpressionParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 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. @@ -28,6 +28,7 @@ * * @author Andy Clement * @author Juergen Hoeller + * @author Sam Brannen * @since 3.0 */ public class SpelExpressionParser extends TemplateAwareExpressionParser { @@ -53,6 +54,7 @@ public SpelExpressionParser(SpelParserConfiguration configuration) { public SpelExpression parseRaw(String expressionString) throws ParseException { + Assert.hasText(expressionString, "'expressionString' must not be null or blank"); return doParseExpression(expressionString, null); } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/TemplateExpressionParsingTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/TemplateExpressionParsingTests.java index 937b54e5c08a..b7ac25c82b9e 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/TemplateExpressionParsingTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/TemplateExpressionParsingTests.java @@ -30,6 +30,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** * @author Andy Clement @@ -43,6 +44,12 @@ class TemplateExpressionParsingTests extends AbstractExpressionTests { private final SpelExpressionParser parser = new SpelExpressionParser(); + @Test + void nullTemplateExpressionIsRejected() { + assertThatIllegalArgumentException() + .isThrownBy(() -> parser.parseExpression(null, DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT)) + .withMessage("'expressionString' must not be null"); + } @Test void parsingSimpleTemplateExpression01() { diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelParserTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelParserTests.java index dfef1d4b85af..33b6e7298e23 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelParserTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelParserTests.java @@ -33,6 +33,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.springframework.expression.spel.SpelMessage.MISSING_CONSTRUCTOR_ARGS; import static org.springframework.expression.spel.SpelMessage.NON_TERMINATING_DOUBLE_QUOTED_STRING; import static org.springframework.expression.spel.SpelMessage.NON_TERMINATING_QUOTED_STRING; @@ -53,6 +54,32 @@ class SpelParserTests { private final SpelExpressionParser parser = new SpelExpressionParser(); + @Test + void nullExpressionIsRejected() { + assertNullOrEmptyExpressionIsRejected(() -> parser.parseExpression(null)); + assertNullOrEmptyExpressionIsRejected(() -> parser.parseRaw(null)); + } + + @Test + void emptyExpressionIsRejected() { + assertNullOrEmptyExpressionIsRejected(() -> parser.parseExpression("")); + assertNullOrEmptyExpressionIsRejected(() -> parser.parseRaw("")); + } + + @Test + void blankExpressionIsRejected() { + assertNullOrEmptyExpressionIsRejected(() -> parser.parseExpression(" ")); + assertNullOrEmptyExpressionIsRejected(() -> parser.parseExpression("\t\n")); + assertNullOrEmptyExpressionIsRejected(() -> parser.parseRaw(" ")); + assertNullOrEmptyExpressionIsRejected(() -> parser.parseRaw("\t\n")); + } + + private static void assertNullOrEmptyExpressionIsRejected(ThrowingCallable throwingCallable) { + assertThatIllegalArgumentException() + .isThrownBy(throwingCallable) + .withMessage("'expressionString' must not be null or blank"); + } + @Test void theMostBasic() { SpelExpression expr = parser.parseRaw("2");