Add LimitBodySizeMiddleware

[Kludex]

• Alternative to Add RequestSizeLimitMiddleware and RequestTimeoutMiddleware #2328.
• Closes Limit max request size #2155.

Q&A

1. Why not also limit chunks?

Because the chunks the application receive are not the chunks the server is receiving from the client.

[alex-oleshkevich]

Can user set a low body size globally and enlarge it per route? It seems to me that app-level middleware returns 413 response as fast as limit reached.

Starlette(
	middleware=[
		Middleware(LimitRequestMiddleware, max_body_size=1024) # 1024b globally
	],
    routes=[
        Route('/'), # it's ok to use a global limit here
		Route('/upload', middleware=[
			Middleware(LimitRequestMiddleware, max_body_size=1024 ** 3 ), # i want a greater limit here
		])
    ],
)

[Kludex]

Can user set a low body size globally and enlarge it per route? It seems to me that app-level middleware returns 413 response as fast as limit reached.

Starlette(
	middleware=[
		Middleware(LimitRequestMiddleware, max_body_size=1024) # 1024b globally
	],
    routes=[
        Route('/'), # it's ok to use a global limit here
		Route('/upload', middleware=[
			Middleware(LimitRequestMiddleware, max_body_size=1024 ** 3 ), # i want a greater limit here
		])
    ],
)

Yes. That should be possible. I'm going to add a test for it.

[adriangb]

Yes. That should be possible. I'm going to add a test for it.

I think that to make have to split it into two middlewares and pass the max size around in the scope like #2175 does

[Kludex]

Ah sorry, I thought you meant to increase the constraint, not to relax it. I don't think we should allow that.

One could always mount the applications as they want.

[adriangb]

Ah sorry, I thought you meant to increase the constraint, not to relax it. I don't think we should allow that.

One could always mount the applications as they want.

I disagree and I think @alex-oleshkevich will as well. Small limits by default and larger limits on certain endpoints seems like a very important use case.

[Kludex]

Ah sorry, I thought you meant to increase the constraint, not to relax it. I don't think we should allow that.
One could always mount the applications as they want.

I disagree and I think @alex-oleshkevich will as well. Small limits by default and larger limits on certain endpoints seems like a very important use case.

I've complied with this.

[Kludex]

I feel like this middleware will give the false impression of security. The moment the application calls receive() is when we are going to do the appropriate checks on the middleware. The thing is that by that moment, the server could already have stored the whole request body in memory.

receive() just gets what is already in memory.

I think a more correct approach would be to restrict this from the server side, and allow customizing per path e.g.:
uvicorn.run(app, limit_max_body_size=2*MB, limit_max_body_size_per_path={'/potato': 3*MB}).

[alex-oleshkevich]

IMO servers should (and do?) stream data and not buffer it for too long. So server is also not aware of the payload size until EOF.

[abersheeran]

I feel like this middleware will give the false impression of security. The moment the application calls is when we are going to do the appropriate checks on the middleware. The thing is that by that moment, the server could already have stored the whole request body in memory.receive()

receive() just gets what is already in memory.

I think a more correct approach would be to restrict this from the server side, and allow customizing per path e.g.: .uvicorn.run(app, limit_max_body_size=2*MB, limit_max_body_size_per_path={'/potato': 3*MB})

I agree this. Add limit body size config in uvicorn/nginx is better.

[Kludex]

After a lot of thinking, I don't think Starlette should have this level of granularity on those limits. I can be convinced on the global level, but not on per endpoint basis. Besides, there's no record of a popular web framework doing this per basis (unless you can prove me wrong?).

We can document that any ASGI web server (Uvicorn and Gunicorn) has global limits set by default.

Also, I do believe it's a nice contribution to have this middleware implemented as a third party. @adriangb Since the last commit is pretty much your implementation, do you mind if I create this package, or do you want to create it yourself?