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 1/2] 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 2356b6011c..bedd0c0f53 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 379ca5b222..cfab5009f7 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 @@ class Tokenizer { 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 48e0b06211..8942718d39 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 4eebccab49..88d2a0718b 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 @@ package org.springframework.expression.spel; 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 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException; * @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 @@ class ParsingTests { 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 From fb0a108254b6634035232e29528cd9eeb1070c0c Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Tue, 22 Oct 2024 13:04:29 +0200 Subject: [PATCH 2/2] Improve Javadoc for core SpEL APIs --- .../expression/ExpressionParser.java | 25 +++++++++++-------- .../expression/ParserContext.java | 9 ++++--- .../common/TemplateAwareExpressionParser.java | 11 +++++--- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/ExpressionParser.java b/spring-expression/src/main/java/org/springframework/expression/ExpressionParser.java index c90dc073bd..a8e93f8cf6 100644 --- a/spring-expression/src/main/java/org/springframework/expression/ExpressionParser.java +++ b/spring-expression/src/main/java/org/springframework/expression/ExpressionParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2024 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. @@ -18,7 +18,8 @@ package org.springframework.expression; /** * Parses expression strings into compiled expressions that can be evaluated. - * Supports parsing templates as well as standard expression strings. + * + *

Supports parsing template expressions as well as standard expression strings. * * @author Keith Donald * @author Andy Clement @@ -27,29 +28,31 @@ package org.springframework.expression; public interface ExpressionParser { /** - * Parse the expression string and return an Expression object you can use for repeated evaluation. - *

Some examples: + * Parse the expression string and return an {@link Expression} object that + * can be used for repeated evaluation. + *

Examples: *

 	 *     3 + 4
 	 *     name.firstName
 	 * 
* @param expressionString the raw expression string to parse - * @return an evaluator for the parsed expression - * @throws ParseException an exception occurred during parsing + * @return an {@code Expression} for the parsed expression + * @throws ParseException if an exception occurred during parsing */ Expression parseExpression(String expressionString) throws ParseException; /** - * Parse the expression string and return an Expression object you can use for repeated evaluation. - *

Some examples: + * Parse the expression string and return an {@link Expression} object that + * can be used for repeated evaluation. + *

Examples: *

 	 *     3 + 4
 	 *     name.firstName
 	 * 
* @param expressionString the raw expression string to parse - * @param context a context for influencing this expression parsing routine (optional) - * @return an evaluator for the parsed expression - * @throws ParseException an exception occurred during parsing + * @param context a context for influencing the expression parsing routine + * @return an {@code Expression} for the parsed expression + * @throws ParseException if an exception occurred during parsing */ Expression parseExpression(String expressionString, ParserContext context) throws ParseException; diff --git a/spring-expression/src/main/java/org/springframework/expression/ParserContext.java b/spring-expression/src/main/java/org/springframework/expression/ParserContext.java index d8504f1a19..4d8fc5f422 100644 --- a/spring-expression/src/main/java/org/springframework/expression/ParserContext.java +++ b/spring-expression/src/main/java/org/springframework/expression/ParserContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -47,7 +47,7 @@ public interface ParserContext { String getExpressionPrefix(); /** - * For template expressions, return the prefix that identifies the end of an + * For template expressions, returns the prefix that identifies the end of an * expression block within a string. For example: "}" * @return the suffix that identifies the end of an expression */ @@ -55,8 +55,9 @@ public interface ParserContext { /** - * The default ParserContext implementation that enables template expression - * parsing mode. The expression prefix is "#{" and the expression suffix is "}". + * The default {@link ParserContext} implementation that enables template + * expression parsing. + *

The expression prefix is "#{", and the expression suffix is "}". * @see #isTemplate() */ ParserContext TEMPLATE_EXPRESSION = new ParserContext() { 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 f979a258fb..bdf4ad1660 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 @@ -29,8 +29,11 @@ import org.springframework.lang.Nullable; import org.springframework.util.Assert; /** - * An expression parser that understands templates. It can be subclassed by expression - * parsers that do not offer first class support for templating. + * Abstract base class for {@linkplain ExpressionParser expression parsers} that + * support templates. + * + *

Can be subclassed by expression parsers that offer first class support for + * templating. * * @author Keith Donald * @author Juergen Hoeller @@ -88,7 +91,7 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser * single quote '. * @param expressionString the expression string * @return the parsed expressions - * @throws ParseException when the expressions cannot be parsed + * @throws ParseException if the expressions cannot be parsed */ private Expression[] parseExpressions(String expressionString, ParserContext context) throws ParseException { List expressions = new ArrayList<>(); @@ -229,7 +232,7 @@ public abstract class TemplateAwareExpressionParser implements ExpressionParser * @param expressionString the raw expression string to parse * @param context a context for influencing this expression parsing routine (optional) * @return an evaluator for the parsed expression - * @throws ParseException an exception occurred during parsing + * @throws ParseException if an exception occurred during parsing */ protected abstract Expression doParseExpression(String expressionString, @Nullable ParserContext context) throws ParseException;