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

Serialization of double-linked-list of tokens breaks recursion limit #1164

Open
darvanen opened this issue Jun 8, 2022 · 3 comments
Open
Labels

Comments

@darvanen
Copy link

darvanen commented Jun 8, 2022

The output of \GraphQL\Language\Parser::parse contains a tree in every 'loc' field which appears to branch forever:

image

Now, it doesn't go forever or it would crash for everyone. But get a schema large enough and it does break the recursion limit (default 4096) in unserialize() when being pulled out of cache (in Drupal).

What purpose do these endless recursive references to 'prev' and 'next' serve? Is it possible they could be limited or removed?

@darvanen
Copy link
Author

darvanen commented Jun 8, 2022

Or perhaps could we come up with a __serialize() member function to \GraphQL\Language\Token to teach the PHP serializer how to handle it?

@spawnia spawnia changed the title Endless recursion in AST Serialization of double-linked-list of tokens breaks recursion limit Jun 8, 2022
@spawnia spawnia added the bug label Jun 8, 2022
@spawnia
Copy link
Collaborator

spawnia commented Jun 8, 2022

I am going to categorize this as a bug, as I believe that serialization of AST nodes should be supported with default PHP settings.

A quick fix for you would be to use the parser option noLocation and bypass this issue. If you do not need the location, this is more efficient.

What purpose do these endless recursive references to 'prev' and 'next' serve? Is it possible they could be limited or removed?

They are neither endless, nor recursive. The tokens form a double-linked-list. I think this data structure was probably copied from the reference implementation graphql-js. It may be possible to optimize this by using something more well suited for PHP.

Or perhaps could we come up with a __serialize() member function to \GraphQL\Language\Token to teach the PHP serializer how to handle it?

I could see this working too, sure. Traversing the double-linked-list can most certainly be transformed into an iterative algorithm.

@darvanen
Copy link
Author

darvanen commented Jun 9, 2022

Thanks for the explanation :)

I'm afraid this pattern is very new to me (obviously) so I probably won't be able to help very much but I will if I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants