Bug: Content Length Check on the Request Body Incompatible with Decompression Middleware

[enerqi]

Description

The connection.request.Request class has a stream method that is used to populate the request's body. It will raise a ClientException("Malformed request") when it detects total_bytes_streamed > announced_content_length.

This does not account for the situation where the ASGI receive function includes decompression middleware in the asgi app chain. When decompressed by middleware the body would naturally be larger than the content length of the network message.

The comments in request.py suggest that the check is somewhat redundant because the ASGI server, such as uvicorn, should check this. As total_bytes_streamed > announced_content_length could be true before the body has finished streaming in and decompressing, would it be better to just remove the check if there is Content-Encoding header present that is set to anything other than identity? Alternatively, if the decompression middleware mutates the content-length header as it goes we could reload the announced_content_length for another check before raising the ClientException.

A version of decompression middleware without gzip support

At this point I'm unclear if this is really a bug or if there's some other approach to avoid the issue.

URL to code causing the issue

No response

MCVE

Steps to reproduce

Send a network request to a Litestar app with a compressed body and a content-encoding header that decompression middleware can use to choose how to decompress that body.

Without the decompression middleware the types won't match the API, it'll just be a byte stream and not e.g json.

Screenshots

No response

Logs

Litestar Version

2.15.2

Platform

• Linux
• Mac
• Windows
• Other (Please specify in the description above)

[provinzkraut]

The comments in request.py suggest that the check is somewhat redundant because the ASGI server, such as uvicorn, should check this

As the comment explains, since this is not part of the ASGI spec, we cannot safely rely on this behaviour.

---

This behaviour is an interesting case, and it first I thought could be solved by moving the check higher up the stack, but the issue is that we're doing two checks in one here:

1. Check if the content-length header matches the actual request body size
2. Check if the request body size exceeds the defined limits

Check 1 could easily be moved to be applied before any user-defined middleware, but the second check in particular is something you probably do not want to apply after decompression, since doing so would create an attack surface for a DoS.

You could easily create a very small, compressed request body that uncompresses to a a much larger size (100MB worth of 0s compress to ~100kB when gzipped), so you could have your limit set to 1MB, and still end up with 100MB worth of junk data in memory from a 100kB request.

I think the ideal way here would be if ensuring content-length matches the body size would be part of the ASGI spec (it is for WSGI), so we could omit this, and then leave the other check in place.

Another solution would be to somehow split the two, e.g. one middleware that counts the bytes read and checks for consistency with content-length, and stores that data in the scope, and the Request.stream then checks if the "bytes read" on the current request exceed its limits.