Customising the maximum size of request payload

[mykola-mokhnach]

🐣 Is your feature request related to a problem? Please describe.

I would like to explicitly limit the maximum size of an incoming json body for a POST request.

💡 Describe the solution you'd like

Currently I see there is a private _client_max_size property for such purpose, which equals to 4MB by default. Although, I don't see a clear way to, for example, limit the maximum size of a json object that one retrieves from await request.json() call.

❓ Describe alternatives you've considered

Currently the following workaround is used:

        default_client_max_size = request._client_max_size
        request._client_max_size = MAX_PAYLOAD_SIZE
        try:
            req_json = await request.json()
        except web_exceptions.HTTPClientError:
            raise
        except Exception:
            raise web.HTTPBadRequest(reason='The request body is expected to be a valid JSON')
        finally:
            request._client_max_size = default_client_max_size

Although, it would be nice if _client_max_size would be public/had a setter, so one could limit the body size in request.text() or request.json() APIs without changing values of private properties.

📋 Additional context

[anesabml]

@mykola-mokhnach there's a way to change the maximum size of request payload when initializing web.Application:
web.Application(client_max_size=1024 ** 2), the specified size will be used for all requests.

[mykola-mokhnach]

Thanks @anesabml My feature request is about customizing it for a single route rather than for the whole webapp

[anesabml]

@mykola-mokhnach I think it is possible to add a feature to specify the maximum size per request. but for now, you can specify it globally or using your suggested workaround.

[webknjaz]

It sounds like you'd want to have a check after receiving an HTTP request. I do not think it's reasonable for it to be present on the HTTP protocol level because route handlers are matched on a higher level than HTTP. For per-handler/route checks, I imagine that the best solution is to make a decorator or a middleware that would look at the Content-Length header and raise an error if it's not satisfactory. You could also use lower level methods to read only a part of the payload to cover the case without the length header.

P.S. Your workaround looks dangerously unpredictable, race-condition-wise: you have await which pauses the coroutine execution and let's other coroutines on the loop get executed, meaning that another request handler may mess with that attribute concurrently.

[mykola-mokhnach]

Your workaround looks dangerously unpredictable, race-condition-wise: you have await which pauses the coroutine execution and let's other coroutines on the loop get executed, meaning that another request handler may mess with that attribute concurrently.

Isn't this property set per request instance? I don't think it's shared with anything else.

For per-handler/route checks, I imagine that the best solution is to make a decorator or a middleware that would look at the Content-Length header and raise an error if it's not satisfactory. You could also use lower level methods to read only a part of the payload to cover the case without the length header.

Why one should trust Content-Length? Nothing prevents an attacher to manipulate its value while providing a different length of the body payload.

You could also use lower level methods to read only a part of the payload to cover the case without the length header.

Yes, I could use read(n_bytes) like it's done in request.json() or request.text() helpers themselves. But then what is the point in having these shortcuts at all?

[webknjaz]

Isn't this property set per request instance?

Ah. I missed that.

Why one should trust Content-Length? Nothing prevents an attacher to manipulate its value while providing a different length of the body payload.

Why would you perceive this as a security feature?

Yes, I could use read(n_bytes) like it's done in request.json() or request.text() helpers themselves. But then what is the point in having these shortcuts at all?

There are always common utility helpers that most people need and then there are features that only one user on the internet will ever use. The latter ain't worthy of being shipped to every single user as a part of the common framework, they should be provided via third-party libs or even embedded into one's project.

That said, for this to be considered for inclusion, we need someone to write up a comparison of how and if other web frameworks provide such an API.

[mykola-mokhnach]

This is, for example, what Django has:
https://docs.djangoproject.com/en/1.11/ref/settings/#data-upload-max-memory-size
https://docs.djangoproject.com/en/1.11/ref/settings/#std:setting-FILE_UPLOAD_MAX_MEMORY_SIZE
django/channels#1240

[webknjaz]

This is, for example, what Django has:
docs.djangoproject.com/en/1.11/ref/settings/#data-upload-max-memory-size
docs.djangoproject.com/en/1.11/ref/settings/#std:setting-FILE_UPLOAD_MAX_MEMORY_SIZE
django/channels#1240

Those settings look global, not per-handler.

[asvetlov]

We have the request.clone() method to change some request properties.

I can accept a pull request that

1. adds client_max_size to a list of accepted arguments
2. adds BaseRequest.client_max_size read-only property
3. reflects changes in docs
4. has some tests to cover new functionality