-
-
Notifications
You must be signed in to change notification settings - Fork 931
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
Limit max request size #2155
Comments
If we're going to make it a configurable middleware it might also make sense to have some sort of timeout for connections and each chunk, maybe infinite by default but definitely tunable. Another thing to keep in mind is that this is likely something users want to control on a per-endpoint basis. That is, if I have an app that has an upload feature where I'm expecting 1GB files it's likely a single endpoint that expects 1GB files so I'd want to bump up the limits just for that endpoint. That makes me think that the best strategy may be a per-endpoint middleware w/ a companion middleware that just tweaks the config by changing it in |
This is a good one! I also agree that we need a global setting and per-route (Route + Mount). We can add |
Why should the ASGI application be the one to set this instead of the server? |
Example: global POST limit is 1mb, for selected endpoints that upload files - 100mb. |
Adding a LimitRequestSizeMiddleware is the simplest and forward-compatible way. |
Yeah. Shall we follow this path? |
Yes I think someone should make a PR and we can discuss the details (override vs. min/max, should there be a default, etc.) there. |
I am someone, I made a PR 😆 : #2328 |
Discussed in #1516
Originally posted by aviramha April 5, 2020
As discussed in the Gitter, my opinion is that starlette should provide a default limit for request size.
The main reason is that without it, any Starlette application is vulnerable to very easy DoS.
For example, newbie me can write a program as follows:
As a malicious user, I could send a 30GB sized JSON and cause the memory to go OOM.
Other frameworks support this also - Django, Quart.
My proposal is to add a default limit which can be overrided in the app configuration.
Important
The text was updated successfully, but these errors were encountered: