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

Support validation "maxErrors" option #8013

Open
mo4islona opened this issue Jan 8, 2025 · 3 comments · May be fixed by #8014
Open

Support validation "maxErrors" option #8013

mo4islona opened this issue Jan 8, 2025 · 3 comments · May be fixed by #8014

Comments

@mo4islona
Copy link

mo4islona commented Jan 8, 2025

We faced an DoS attack which simply was exploiting invalid requests like

query AAA ($a:a, $a:a, $a:a, $a:a, $a:a, $a:a, $a:a, $a:a, ..... 1000 elems..., $a:a, ) { a }
query AAB ($b:b, $a:a, $a:a, $a:a, $a:a, $a:a, $a:a, $a:a, ..... 1000 elems..., $a:a, ) { a }
query AAC ($c:c, $a:a, $a:a, $a:a, $a:a, $a:a, $a:a, $a:a, ..... 1000 elems..., $a:a, ) { a }

It it produces huge latency and blocks other concurrent queries (NodeJS loop is blocked by validating and/or errors generating) which is the most crucial point.

Graphql validate function has an option to configure maxErrors and aborts the validation if it hits the limit.

Before (unlimited, maxErrors: undefined)

image

After (maxErrors: 10)

image

@mo4islona mo4islona linked a pull request Jan 8, 2025 that will close this issue
@glasser
Copy link
Member

glasser commented Jan 8, 2025

I'm surprised to see you're saying the default is unlimited — isn't the default in graphql-js (or versions with the maxErrors flag) 100?

@mo4islona
Copy link
Author

mo4islona commented Jan 8, 2025

Yep, you are right. Sorry for the confusion.

We haven't migrated yet and I tested a quick patch to outdated [email protected] and [email protected]

https://github.com/graphql/graphql-js/blob/15.x.x/src/validation/validate.js#L38

The latest [email protected] changed the default value to 100.

@mo4islona
Copy link
Author

It is much better with 100, but anyway I'd like to be able to configure this.
The difference is very noticeable on not very performant cores.

image

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 a pull request may close this issue.

2 participants