spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.75k stars 38.15k forks source link

Throw `SpelParseException` for unsupported character in `SpelExpressionParser` #33767

Closed MikailBag closed 1 month ago

MikailBag commented 1 month ago

Reproducer: https://gist.github.com/MikailBag/ed3b036da394aae7c57e89267920f522

Actual expression: "#{m[‘c]}"

(Please note that ‘ is not an ASCII quote)

Expected behavior: program prints "OK" (or maybe "Incorrect success", if the expression is actually correct).

Actual behavior:

> Task :Main.main() FAILED
Exception in thread "main" java.lang.IllegalStateException: Unsupported character '‘' (8216) encountered at position 3 in expression.
    at org.springframework.expression.spel.standard.Tokenizer.process(Tokenizer.java:271)
    at org.springframework.expression.spel.standard.InternalSpelExpressionParser.doParseExpression(InternalSpelExpressionParser.java:135)
    at org.springframework.expression.spel.standard.SpelExpressionParser.doParseExpression(SpelExpressionParser.java:63)
    at org.springframework.expression.spel.standard.SpelExpressionParser.doParseExpression(SpelExpressionParser.java:34)
    at org.springframework.expression.common.TemplateAwareExpressionParser.parseExpressions(TemplateAwareExpressionParser.java:125)
    at org.springframework.expression.common.TemplateAwareExpressionParser.parseTemplate(TemplateAwareExpressionParser.java:66)
    at org.springframework.expression.common.TemplateAwareExpressionParser.parseExpression(TemplateAwareExpressionParser.java:52)
    at Main.main(Main.java:10)

Execution failed for task ':Main.main()'.
> Process 'command '/usr/lib/jvm/java-21-openjdk-amd64/bin/java'' finished with non-zero exit value 1

I consider this behavior incorrect, as SpelExpressionParser is documented to throw (checked) ParseException, and not documented to throw any runtime exceptions (except maybe NullPointerException if input is null).

This issue may be related to #31097.

I was able to reproduce the problem with the latest published stable version v6.1.14.

sbrannen commented 1 month ago

Hi @MikailBag,

Congratulations on submitting your first issue for the Spring Framework! 👍

I consider this behavior incorrect, as SpelExpressionParser is documented to throw (checked) ParseException, and not documented to throw any runtime exceptions (except maybe NullPointerException if input is null).

org.springframework.expression.ParseException is actually an unchecked RuntimeException.

However, I agree that throwing a ParseException (or even better a SpelParseException) for an unsupported character would be better than throwing the IllegalStateException, and I've updated the title of this issue to reflect that.

Cheers,

Sam

sbrannen commented 1 month ago

This has been fixed in c98f314665a27f88dbb8168db650c1598cafad72 for inclusion in the upcoming Spring Framework 6.1.15 release.