Skip to content

Prevent stack overflow crashes with recursion limit#21

Merged
mattt merged 9 commits intotoon-format:mainfrom
rickhohler:feat/recursion-protection
Feb 6, 2026
Merged

Prevent stack overflow crashes with recursion limit#21
mattt merged 9 commits intotoon-format:mainfrom
rickhohler:feat/recursion-protection

Conversation

@rickhohler
Copy link
Copy Markdown
Contributor

This PR addresses a missing safety mechanism in TOONEncoder where deeply nested structures could cause a stack overflow crash.

Changes

  • Added recursionLimit: New property on TOONEncoder (default: 1000).
  • Depth Checking: Implemented depth checks in Keyed, Unkeyed, and SingleValue encoding containers.
  • Error Handling: Instead of crashing, the encoder now throws EncodingError.invalidValue if the recursion limit is exceeded.

Verification

  • Enabled testDeepRecursion in BugHuntingTests which confirms that the encoder now throws a catchable error instead of crashing the process.

Note: testValueFromDictionaryKeyOrder and testNegativeZeroEncoding in BugHuntingTests will fail on this branch as they depend on the fixes in PR #20 (Serialization Correctness). This is expected isolation.

Copy link
Copy Markdown
Collaborator

@mattt mattt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening this PR, @rickhohler! Left some comments with suggested changes.

@mattt mattt changed the title fix: Prevent stack overflow crashes with recursion limit Prevent stack overflow crashes with recursion limit Feb 5, 2026
@rickhohler rickhohler force-pushed the feat/recursion-protection branch from 7f74155 to 5c5ebf3 Compare February 5, 2026 16:11
@mattt
Copy link
Copy Markdown
Collaborator

mattt commented Feb 6, 2026

Thanks for your help with this, @rickhohler! I worked through the remaining feedback and adjusted the recursion limit to 32, matching the decoding limit.

@mattt mattt merged commit 2d24224 into toon-format:main Feb 6, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants