Skip to content

Enforce a recursion-depth ceiling in json.dumps serialization#438

Closed
dsp-ant wants to merge 1 commit into
pydantic:mainfrom
dsp-ant:fix/json-dumps-recursion-limit
Closed

Enforce a recursion-depth ceiling in json.dumps serialization#438
dsp-ant wants to merge 1 commit into
pydantic:mainfrom
dsp-ant:fix/json-dumps-recursion-limit

Conversation

@dsp-ant
Copy link
Copy Markdown
Contributor

@dsp-ant dsp-ant commented May 14, 2026

The serialize_value/serialize_sequence/serialize_dict helpers are mutually recursive and run entirely within one bytecode instruction, so the VM's per-instruction recursion check never fires. The cycle detector catches reference loops but not deep acyclic chains, which can overflow the host's native stack.

Add a depth ceiling matching the JSON_RECURSION_LIMIT already used by json.loads, raising a catchable RecursionError once exceeded.


Summary by cubic

Adds a recursion-depth ceiling to json.dumps to prevent native stack overflows on deeply nested acyclic structures. Matches the json.loads limit and raises a catchable RecursionError when exceeded.

  • Bug Fixes
    • Enforce JSON_DUMP_RECURSION_LIMIT (200) in the serializer and map overflow to RecursionError.
    • Add tests: deep nesting triggers the error; moderate nesting stays within the limit.

Written for commit 0582012. Summary will update on new commits.

The serialize_value/serialize_sequence/serialize_dict helpers are
mutually recursive and run entirely within one bytecode instruction,
so the VM's per-instruction recursion check never fires. The cycle
detector catches reference loops but not deep acyclic chains, which
can overflow the host's native stack.

Add a depth ceiling matching the JSON_RECURSION_LIMIT already used by
json.loads, raising a catchable RecursionError once exceeded.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 14, 2026

Merging this PR will not alter performance

✅ 17 untouched benchmarks
⏩ 15 skipped benchmarks1


Comparing dsp-ant:fix/json-dumps-recursion-limit (0582012) with main (ac5b383)

Open in CodSpeed

Footnotes

  1. 15 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Comment on lines +370 to +377
/// Maximum nesting depth accepted by `json.dumps()`.
///
/// Mirrors `JSON_RECURSION_LIMIT` on the `json.loads()` side: serialization is
/// mutually recursive across `serialize_value`/`serialize_sequence`/`serialize_dict`
/// and would otherwise overflow the host's native stack on a deep (acyclic)
/// structure that the cycle detector cannot catch. CPython raises a catchable
/// `RecursionError` here; we map the depth-limit `ResourceError` to the same.
const JSON_DUMP_RECURSION_LIMIT: usize = 200;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why does this use a constant rather than respect the ResourceLimit recursion limit?

Separately, it seems like CPython does not, a 20000-deep nested tuple will serialize fine even if sys.setrecursionlimit is set very low.

Seems like CPython uses stack overflow protections for these cases, if I write a million element tuple I eventually get

RecursionError: Stack overflow (used 8144 kB) while encoding a JSON object

... maybe #440?

@davidhewitt
Copy link
Copy Markdown
Collaborator

I decided to solve this case with the general recursion limit mechanism for now, see #454

Thanks again for the report & patch, will close this one.

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