Skip to content

Commit

Permalink
Reject null and empty SpEL expressions
Browse files Browse the repository at this point in the history
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.

See gh-30371
Closes gh-30373
  • Loading branch information
sbrannen committed Apr 25, 2023
1 parent 5afd94f commit 964950a
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -28,6 +28,7 @@
*
* @author Andy Clement
* @author Juergen Hoeller
* @author Sam Brannen
* @since 3.0
*/
public class SpelExpressionParser extends TemplateAwareExpressionParser {
Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -72,6 +73,13 @@ public boolean isTemplate() {
private final SpelExpressionParser parser = new SpelExpressionParser();


@Test
void nullTemplateExpressionIsRejected() {
assertThatIllegalArgumentException()
.isThrownBy(() -> parser.parseExpression(null, DEFAULT_TEMPLATE_PARSER_CONTEXT))
.withMessage("'expressionString' must not be null");
}

@Test
void parsingSimpleTemplateExpression01() {
Expression expr = parser.parseExpression("hello ${'world'}", DEFAULT_TEMPLATE_PARSER_CONTEXT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
Expand Down

0 comments on commit 964950a

Please sign in to comment.