fix: guard sanitize_for_json against circular references - #515
Open
SarthakB11 wants to merge 1 commit into
Open
Conversation
Recursive containers (e.g. an object graph with a back-reference) hit Python's recursion limit and crash with RecursionError instead of returning a usable (if lossy) JSON payload. Track visited container ids per recursion path and substitute a placeholder on repeat instead of recursing forever. Closes sktime#189
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes #189.
Problem
sanitize_for_jsonrecurses into dicts/lists/tuples/Series/DataFrameswith no cycle detection. An object graph containing a back-reference
(e.g. a dict that (indirectly) contains itself) blows the recursion
limit and crashes with
RecursionErrorinstead of returning anyusable response to the calling tool.
Fix
Track the
id()of containers visited along the current recursionpath in a
_seenset (passed as an internal-only kwarg, not part ofthe public signature). On revisiting an already-seen container id,
return the placeholder string
"<circular reference>"instead ofrecursing again. Each recursive branch gets its own copy of
_seen(immutable set union), so sibling branches that legitimately share the
same nested object are not falsely flagged as circular.
Tests
Added 4 regression tests to
tests/test_sanitize.py:a -> b -> a)Full suite passes locally: 201 passed.