Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect circular references and return early #69

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

iamvery
Copy link

@iamvery iamvery commented Aug 17, 2017

I ran into this issue today while dealing with a large object tree that contained circular references. Due to the recursive nature of the path I was using, the result was a RangeError: Maximum call stack size exceeded. For my particular use case, it would be preferable for jsonpath to be resilient to such cases by avoiding traversing circular references again.

My solution is keep a set of visited objects as the tree is traversed. If the same object shows up twice then descend avoids visiting it again.

Hope you find this useful! Let me know if somethings needs more work ❤️

This corrects an issue where non-circular references were falsely
detected as _all_ seen objects were stored across the entire recursion.
@iamvery
Copy link
Author

iamvery commented Aug 17, 2017

It looks like the version of Node used in CI doesn't support Set, so I just pushed a commit that uses a (slower) Array implementation. Not sure if upgrading Node is an option. Let me know!

@jeremy-w
Copy link

It looks like this handles references in objects. What happens if the circular reference occurs within an array?

var containsItself = ['value'];
containsItself.push(containsItself);
var results = jp.query(obj, '$..*');

@iamvery
Copy link
Author

iamvery commented Aug 17, 2017

Nice catch!

Unfortunately there seems to be some other more subtle issue as well. I have yet to reproduce it in this lib's test suite, but in my application code the terminal completely hangs when traversing the problematic object. Interestingly, when I remove the "fix" I added to account for non-circular repetition then things run fine... Where's the loop??? 🤔

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