Add configurable structural nesting depth limit to SpEL expressions#36960
Closed
itsmehotpants wants to merge 1 commit into
Closed
Add configurable structural nesting depth limit to SpEL expressions#36960itsmehotpants wants to merge 1 commit into
itsmehotpants wants to merge 1 commit into
Conversation
Closes spring-projects#36723 SpelParserConfiguration already enforces a maximum expression length and a maximum number of evaluation operations, but had no corresponding guard on structural nesting depth (e.g. deeply-nested inline lists and maps such as {{{{…}}}}). Without a bound, a crafted expression can drive the recursive-descent parser into arbitrarily deep Java call stacks, leading to StackOverflowError. Changes ------- SpelMessage • New entry MAX_NESTING_DEPTH_EXCEEDED (code 1086) used when the configured limit is breached during parsing. SpelParserConfiguration • DEFAULT_MAX_NESTING_DEPTH = 1_000 constant. • SPRING_EXPRESSION_MAX_NESTING_DEPTH_PROPERTY_NAME system-property constant ('spring.expression.maxNestingDepth') for global override. • maximumNestingDepth field with getter getMaximumNestingDepth(). • New canonical 8-arg constructor that accepts maximumNestingDepth; the previous 7-arg constructor now delegates to it (deprecated). • retrieveMaxNestingDepth() static helper reads the system property with the same validation pattern used by retrieveMaxOperations(). InternalSpelExpressionParser • nestingDepth counter field, reset to 0 at the start of each parse. • enterNesting(startPos) / exitNesting() helpers that increment / decrement the counter and throw SpelParseException (via internalException) when the limit is exceeded. • maybeEatInlineListOrMap() calls enterNesting() on LCURLY and exitNesting() before pushing the finished node, so every inline list and map contributes exactly one level of depth. SpelNestingDepthTests (new) • 15 JUnit 5 tests covering: configuration defaults, validation, lists, maps, mixed nesting, limit-boundary conditions, parser reusability after an exception, and error-message content.
Member
|
Hi @itsmehotpants, Congratulations on submitting your first PR for the Spring Framework! 👍 Unfortunately, #36723 is assigned to me and not open to contributions. I stated this already in #36723 (comment). In the future, please make sure you read all comments in an issue before spending time crafting and submitting a PR. Regards, Sam |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Closes #36723
SpelParserConfigurationalready enforces a maximum expression length and a maximum number of evaluation operations, but had no corresponding guard on structural nesting depth — e.g. deeply-nested inline lists{{{{…}}}}and maps{'a': {'b': {'c': …}}}. Without a bound, a crafted expression can drive the recursive-descent parser into an arbitrarily deep Java call stack, eventually causingStackOverflowError.This PR adds a configurable
maximumNestingDepthlimit that fires a descriptiveSpelParseExceptionthe moment the parser descends past the threshold.Changes
SpelMessageMAX_NESTING_DEPTH_EXCEEDED(code1086) used when the configured limit is exceeded during parsing.SpelParserConfigurationDEFAULT_MAX_NESTING_DEPTH = 1_000— conservative default that is safe for any legitimate expression.SPRING_EXPRESSION_MAX_NESTING_DEPTH_PROPERTY_NAME(spring.expression.maxNestingDepth) — system property /SpringPropertieskey for global override, consistent with the existingmaxOperationspattern.maximumNestingDepthfield +getMaximumNestingDepth()accessor.maximumNestingDepth; the previous 7-arg constructor delegates to it and is marked@Deprecated(since = "7.0").retrieveMaxNestingDepth()static helper follows exactly the same validation pattern asretrieveMaxOperations().InternalSpelExpressionParsernestingDepthcounter field, reset to0at the start of everydoParseExpressioncall (ensuring parser instances are safely reusable).enterNesting(startPos)— increments the counter and throws viainternalExceptionif the limit is exceeded.exitNesting()— decrements the counter.maybeEatInlineListOrMap()— callsenterNesting()onLCURLYandexitNesting()just before the finished node is pushed, so every inline list and map contributes exactly one depth level.SpelNestingDepthTests(new)15 JUnit 5 tests covering:
Usage
Checklist
maximumExpressionLength,maximumOperations)spring.expression.maxNestingDepth)@Deprecated(since = "7.0")on the superseded 7-arg constructor