From c98f314665a27f88dbb8168db650c1598cafad72 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Tue, 22 Oct 2024 13:03:08 +0200 Subject: [PATCH] Throw ParseException for unsupported character in SpEL expression Prior to this commit, the SpEL Tokenizer threw an IllegalStateException when an unsupported character was detected in a SpEL expression; however, the Javadoc for ExpressionParser.parseExpression() states that it throws a ParseException if an exception occurred during parsing. This commit addresses that issue by throwing a SpelParseException for an unsupported character in a SpEL expression, using a new UNSUPPORTED_CHARACTER enum constant in SpelMessage. Closes gh-33767 --- .../expression/spel/SpelMessage.java | 6 ++++- .../expression/spel/standard/Tokenizer.java | 4 +--- .../spel/AbstractExpressionTests.java | 2 +- .../expression/spel/ParsingTests.java | 23 +++++++++---------- 4 files changed, 18 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 2356b6011c59..bedd0c0f53d8 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 @@ -291,7 +291,11 @@ public enum SpelMessage { /** @since 6.0.13 */ NEGATIVE_REPEATED_TEXT_COUNT(Kind.ERROR, 1081, - "Repeat count ''{0}'' must not be negative"); + "Repeat count ''{0}'' must not be negative"), + + /** @since 6.1.15 */ + UNSUPPORTED_CHARACTER(Kind.ERROR, 1082, + "Unsupported character ''{0}'' ({1}) encountered in expression"); private final Kind kind; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/standard/Tokenizer.java b/spring-expression/src/main/java/org/springframework/expression/spel/standard/Tokenizer.java index 379ca5b22250..cfab5009f79d 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/standard/Tokenizer.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/standard/Tokenizer.java @@ -266,9 +266,7 @@ else if (isTwoCharToken(TokenKind.SAFE_NAVI)) { raiseParseException(this.pos, SpelMessage.UNEXPECTED_ESCAPE_CHAR); break; default: - throw new IllegalStateException( - "Unsupported character '%s' (%d) encountered at position %d in expression." - .formatted(ch, (int) ch, (this.pos + 1))); + raiseParseException(this.pos + 1, SpelMessage.UNSUPPORTED_CHARACTER, ch, (int) ch); } } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/AbstractExpressionTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/AbstractExpressionTests.java index 48e0b062112d..8942718d39fb 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/AbstractExpressionTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/AbstractExpressionTests.java @@ -42,7 +42,7 @@ public abstract class AbstractExpressionTests { protected static final boolean SHOULD_NOT_BE_WRITABLE = false; - protected final ExpressionParser parser = new SpelExpressionParser(); + protected final SpelExpressionParser parser = new SpelExpressionParser(); protected final StandardEvaluationContext context = TestScenarioCreator.getTestEvaluationContext(); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/ParsingTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/ParsingTests.java index 4eebccab494b..88d2a0718bca 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/ParsingTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/ParsingTests.java @@ -18,12 +18,12 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.springframework.expression.spel.standard.SpelExpression; -import org.springframework.expression.spel.standard.SpelExpressionParser; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** * Parse some expressions and check we get the AST we expect. @@ -34,10 +34,7 @@ * @author Andy Clement * @author Sam Brannen */ -class ParsingTests { - - private final SpelExpressionParser parser = new SpelExpressionParser(); - +class ParsingTests extends AbstractExpressionTests { @Nested class Miscellaneous { @@ -104,12 +101,14 @@ void supportedCharactersInIdentifiers() { parseCheck("have乐趣()"); } - @Test - void unsupportedCharactersInIdentifiers() { - // Invalid syntax - assertThatIllegalStateException() - .isThrownBy(() -> parser.parseRaw("apple~banana")) - .withMessage("Unsupported character '~' (126) encountered at position 6 in expression."); + @ParameterizedTest(name = "expression = ''{0}''") + @CsvSource(textBlock = """ + apple~banana, ~, 6 + map[‘c], ‘, 5 + A § B, §, 3 + """) + void unsupportedCharacter(String expression, char ch, int position) { + parseAndCheckError(expression, SpelMessage.UNSUPPORTED_CHARACTER, position, ch, (int) ch); } @Test