Skip to content

fix: bound JSON nesting depth to prevent stack overflow (#540) - #546

Open
slsgzs-cloud wants to merge 3 commits into
wavefnd:masterfrom
slsgzs-cloud:fix-json-depth-limit
Open

fix: bound JSON nesting depth to prevent stack overflow (#540)#546
slsgzs-cloud wants to merge 3 commits into
wavefnd:masterfrom
slsgzs-cloud:fix-json-depth-limit

Conversation

@slsgzs-cloud

@slsgzs-cloud slsgzs-cloud commented Sep 8, 2026

Copy link
Copy Markdown

Summary

Two fixes in this PR:

Fix 1: Bound JSON nesting depth to prevent stack overflow (#540)

parse_value recursively calls parse_array/parse_object without tracking a depth limit. Nested metadata can consume call stack proportional to input nesting.

  • Add MAX_DEPTH = 256 constant and depth: usize field to Parser
  • Check depth >= MAX_DEPTH at the start of parse_array and parse_object
  • Return Err("maximum nesting depth exceeded") when limit reached

Fix 2: Stop classifying valid program stderr as compiler failure (#538)

run_and_classify checked looks_like_fail(result.stderr) before comparing the expected exit status. Any stderr containing phrases such as stack overflow, SyntaxError, or error[E was rejected, even when the program deliberately printed that text and exited with the expected status.

Fix: check exit code first. If the program exits with the expected status, it succeeded — don't reject based on stderr content alone. The compiler/child output is combined, so a real compiler failure would show as a non-zero exit code, not just stderr text.

@LunaStev

LunaStev commented Sep 8, 2026

Copy link
Copy Markdown
Member

The implementation direction looks good, but both #538 and #540 explicitly require regression coverage.

Could you add tests for the acceptance cases before we merge this?

For #540, please cover array/object/alternating nesting around the chosen depth limit and verify overly deep input returns an error rather than crashing.

For #538, please cover successful stderr containing the failure sentinels, expected non-zero exits, and a genuine compiler failure so we know removing the stderr heuristic does not hide producer failures.

@LunaStev
LunaStev requested review from LunaStev and ehcl2048 and removed request for LunaStev and ehcl2048 September 8, 2026 15:18
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