parser: bound recursion depth to avoid stack overflow on deeply nested input - #64371
Karolina G. (kajaaz) wants to merge 2 commits into
Conversation
typescript-automation
Bot
added
the
For Uncommitted Bug
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Some recursive paths remain unbounded, and speculative rewind can discard the new recovery state.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 2
Open (3)
What changed in this PR
Adds parser nesting limits to prevent fatal stack overflows on pathological TypeScript input.
Changes:
- Tracks expression and type nesting depth.
- Emits TS1700 and fast-forwards to EOF at the limit.
- Adds regression tests for deep and moderate nesting.
| File | Description |
|---|---|
tsc/internal/parser/parser.go |
Implements nesting limits and recovery. |
tsc/internal/parser/parser_test.go |
Tests deep-input recovery and valid nesting. |
tsc/internal/diagnostics/diagnostics_generated.go |
Adds generated TS1700 metadata. |
tsc/internal/diagnostics/diagnosticMessages.json |
Defines the new diagnostic. |
Files not reviewed (1)
- tsc/internal/diagnostics/diagnostics_generated.go: Generated file
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| if !p.enterNesting() { | ||
| return p.createMissingTypeNode() | ||
| } | ||
| defer p.leaveNesting() |
| if !p.enterNesting() { | ||
| return p.createMissingIdentifier() | ||
| } | ||
| defer p.leaveNesting() | ||
| return p.parseAssignmentExpressionOrHigherWorker(true /*allowReturnTypeInArrowFunction*/) |
| func (p *Parser) enterNesting() bool { | ||
| if p.nestingDepth >= maxNestingDepth { | ||
| if !p.nestingLimitHit { | ||
| p.parseErrorAtCurrentToken(diagnostics.Expression_or_type_is_too_deeply_nested_Simplify_or_split_it_into_smaller_parts) | ||
| p.nestingLimitHit = true | ||
| p.skipToEndOfFile() | ||
| } |
4a43cb8 to
eda7cec
Compare
|
@microsoft-github-policy-service agree company="Ledger" |
|
We've been suggested to do similar things before; I do not think setting an arbitrary maximum like this will fix things, and as the copilot comments note, it is a bit tricky to get all of the recursion and saving right. I have some other changes I need to send at some point which instead restructure the parser to not need to recurse so much for these cases. |
|
Closing per linked discussion; this PR attempts to enforce an invariant that is documented as explicitly not existing. |
Ref #64370
Problem
The native parser (
tsc/internal/parser) has no nesting-depth cap, so pathologically nested input (e.g. ~1,000,000 nested(,[, orA<) aborts the entire process with a fatal Gostack overflowruntime error. This is not a catchable diagnostic,recover()cannot handle it, so a long-lived host (tsgo --lsp, watch/build daemon, editor host) dies on a single file instead of reporting a parse error and continuing.Fix
Add a bounded recursion counter (
maxNestingDepth = 40000) to the recursive-descent entry points that drive the overflow:parseAssignmentExpressionOrHigher(expressions / array literals)parseType(type arguments)When the limit is first exceeded, the parser reports a single new diagnostic,
TS1700: Expression or type is too deeply nested. Simplify or split it into smaller parts., and fast-forwards the scanner to EOF so the recursive descent unwinds in time linear in the nesting depth. Fast-forwarding to EOF also avoids a pre-existing O(n²) error-recovery path that would otherwise trigger on an unterminatedA<chain (re-speculated as a generic call in expression position).The cap is far above any human-authored or realistic machine-generated source, and well below the depth that exhausts the goroutine stack. Valid, balanced nesting under the cap is unaffected.
Implementation notes:
diagnosticMessages.jsonand the generated file is regenerated (not hand-edited).putParser, so the new fields never leak across source files.Behavior (
tsgo --noEmit)(× 60kstack overflowTS1700, ~0.1s[× 60kstack overflowTS1700, ~0.1sA<× 60kTS1700, ~0.3sTests
Added
TestDeeplyNestedInputDoesNotOverflow(parens, array literals, type arguments) andTestModeratelyNestedInputIsAcceptedintsc/internal/parser/parser_test.go.