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

Max path depth for "Include" #598

Closed
andrus opened this issue Jan 11, 2023 · 0 comments
Closed

Max path depth for "Include" #598

andrus opened this issue Jan 11, 2023 · 0 comments
Milestone

Comments

@andrus
Copy link
Contributor

andrus commented Jan 11, 2023

Agrest (just like GraphQL) is prone to "infinite include" problem unless special measures are taken, when a call specifies a very long (possibly circular) include. Previously we've dealt with it with hardcoded path length limit (300 chars). Let's make it more explicit, and allow the users to specify max allowed path "depth" in terms of the number of relationships.

  • It is a rare parameter that is not attached to an entity, but rather to the entire runtime (or an individual request).
  • Attribute includes will NOT be counted towards "max depth", so it will only control the depth of relationship includes.
  • 0 depth means no relationships, 1 means only direct relationships, etc.
  • A negative value is invalid and causes exceptions
  • There's always a depth policy associated with each request with includes. The default (if not configured explicitly) is 100. Max possible depth is Integer.MAX_VALUE, which should be enough for any practical cases.
@andrus andrus changed the title "relatedIncludeDepth" Include depth Jan 14, 2023
@andrus andrus changed the title Include depth Max include depth Jan 15, 2023
@andrus andrus changed the title Max include depth Max path depth Jan 15, 2023
andrus added a commit that referenced this issue Jan 15, 2023
andrus added a commit that referenced this issue Jan 15, 2023
attaching depth policy to SelectContext
andrus added a commit that referenced this issue Jan 15, 2023
processing the depth policy in the merger
andrus added a commit that referenced this issue Jan 15, 2023
renaming "max include" to "max path" depth
andrus added a commit that referenced this issue Jan 15, 2023
processing the depth policy in the merger
@andrus andrus added this to the 5.0.M10 milestone Jan 15, 2023
@andrus andrus closed this as completed Jan 15, 2023
@andrus andrus changed the title Max path depth Max path depth for "Include" Jan 15, 2023
andrus added a commit that referenced this issue Jan 31, 2023
.. a test that default includes still work
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

No branches or pull requests

1 participant