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

Ability to disable batch request processing #5686

Closed
ilyavolodin opened this issue Sep 1, 2021 · 5 comments
Closed

Ability to disable batch request processing #5686

ilyavolodin opened this issue Sep 1, 2021 · 5 comments
Labels
🧞‍♂️ enhancement ⛲️ feature New addition or enhancement to existing solutions 📚 good-first-issue Issues that are more approachable for first-time contributors.

Comments

@ilyavolodin
Copy link

It would be great to have a built-in ability to disable batch request processing and only allow one mutation/query per request. While batch queries can definitely be helpful in some cases to limit number of round-trips from client to server, they open major vulnerability to DDOS attacks and allow bad actors to make graphql server do their job for them.

One example that comes to mind is 2FA validation code. Let's say your server sends and SMS with 4 digit validation code and you have to enter that code to log into the site. With batch queries, you could create a script that would generate a batch of 9999 mutations with every permutation of 2FA code and one of them is going to be correct. Obviously, things like that should be protected in the code itself, but having an ability to disable batch requests would significantly simplify resolution of this issue, by using standard rate-limiting proxy server such as envoy rate limiter, for example.

@glasser
Copy link
Member

glasser commented Sep 14, 2021

I agree that this would be reasonable. Frankly perhaps you should have to opt in to allowing batch requests, although I don't want to do that for backwards-compatibility reasons. (I find the whole feature to be not very compelling — it's arguably a performance feature, but it means that every operation is as slow as the slowest among them... would rather work on making sure that pipelining/http2 works instead.) Adding a new allowBatchedHttpRequests: false or something to the ApolloServer constructor would be nice. (That data has to be stored on ApolloServer and threaded into GraphQLServerOptions from the graphQLServerOptions method into the annoyingly non-class-method runHttpQuery.) Would love to see a PR!

@glasser glasser added ⛲️ feature New addition or enhancement to existing solutions 📚 good-first-issue Issues that are more approachable for first-time contributors. 🧞‍♂️ enhancement labels Sep 14, 2021
@devkot
Copy link
Contributor

devkot commented Sep 30, 2021

That's a nice suggestion that could prove very useful. I'd like to pick this up - @glasser could you assign this to me please?

@glasser
Copy link
Member

glasser commented Sep 30, 2021

We don't typically use the GitHub assignment feature for folks outside of the core team, but feel free to work on this and send a PR! Looking forward to it.

@glasser
Copy link
Member

glasser commented Nov 2, 2021

Implemented in #5778.

@glasser glasser closed this as completed Nov 2, 2021
@glasser
Copy link
Member

glasser commented Nov 5, 2021

This is released in v3.5.0.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧞‍♂️ enhancement ⛲️ feature New addition or enhancement to existing solutions 📚 good-first-issue Issues that are more approachable for first-time contributors.
Projects
None yet
Development

No branches or pull requests

3 participants