feat: introduce maximum parser recursion depth #1230
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.
Problem
Parsing extremely deeply nested structures (e.g., nested arrays [[[...]]], objects {a:{a:{...}}}, or long chains of unary/binary operators) could lead to excessive recursion in the C++ parser functions, potentially exhausting the native call stack and crashing the interpreter.
Solution
A constant
MAX_PARSER_DEPTH
(set to 1000) is introduced incore/parser.cpp
. Recursive parsing functions (parse
,parseArgs
,parseParams
,parseBind
,parseObjectRemainder
,parseComprehensionSpecs
,parseTerminalBracketsOrUnary
,maybeParseGreedy
,parseInfix
) now track the current recursion depth and throw aStaticError
if this limit is exceeded. This ensures the parser terminates gracefully for pathological inputs instead of crashing.The
current_depth
parameter is generally incremented by 1 whenever a parsing function makes a recursive call to parse a distinct sub-expression or nested structure (e.g., callingparse
from withinparseObjectRemainder
to handle a field's value, or callingparse
fromparseInfix
to handle the right-hand operand). This accurately tracks the syntactic nesting depth. Calls that are part of processing the same structural level (likeparseParams
callingparseArgs
internally) might pass the depth unchanged.A new test case (
test_suite/error.parse.deep_array_nesting.jsonnet
) has been added to verify that the depth limit is correctly enforced and produces the expected static error. This test case uses deeply nested arrays, which demonstrably causes a segmentation fault on the master branch due to native stack overflow during parsing, but is now caught gracefully by the introduced limit.I also updated all function doc strings as they were either missing or outdated.
Why a constant?
Making this limit a constant (rather than a command-line configurable option like
--max-stack
) was a deliberate design choice at this point:MAX_PARSER_DEPTH
guards the parsing stage against C++ stack overflows, while--max-stack
limits the evaluation stage's Jsonnet VM call stack.That being said, this is open for discussion and feedback.
NOTE: originally reported through Google OSS VRP and public disclosure was preferred.