Add server GUnzip middleware for request decompression - #7631

do we need to add TransferEncoding chunked here?

Yeah, you are right, since we are removing Content-Length. Updated.

@hamnis Is there anything else I should adjust or does it look fine to you?

Is this log worth the hassle of propagating the log4cats.LoggerFactory -> log4cats.Logger into API? We won't lose many if we remove it. Alternatively, if we badly want it, let's use org.http4s.Platform#loggerFactory which is spread across the codebase already.

What about using org.http4s.Platform#loggerFactory, as I understood from the discussion in the original PR, this is considered an anti-pattterns and we don't want to use it in new code in favor of org.typelevel.log4cats.LoggerFactory.
Yes, of course we can just remove logging from this middleware at all, but I see a couple of minor issues with such a way:

• We lose an opportunity of investigation by looking at order of middlewares application in logs.
• In the future we won't be able to add logging even if necessary without breaking binary compatibility.

Let's move this const into a final val with proper naming within GUnzip.

Also, it'd be nice to tweak the org.http4s.server.middleware.GZip#apply to use a single default val.

Done both for GUnzip and GZip.

Could you add a test for a "gzip bomb" request, such a request with a body of "a" repeated for a few gigabytes but compressed into something tiny?

Could you please take a look if this is what you had in mind?

Not quite, I would like to see that it returns "request too large" or something similar if the request sent is small (and has a similarly small content-length header), but the decompressed body is much, much larger, and exceeds withMaxRequestLength - I just took that from Armeria, but there should be something similar for other middlewares or requests, I guess? I haven't used http4s in a while, so I don't remember where to look.

Your test is still useful to verify that this middleware lazily decodes potentially infinite request bodies in a streaming manner, but the story will be different with Entity.Strict in http4s 1.x: https://http4s.org/v1/docs/entity.html#why-entity

It is less of an issue when all request bodies are streaming, but could still open up for smuggling in requests that end up being much larger than they should be allowed to be.

Http4s has the separate EntityLimiter middleware for entity size control. I've added one more test for GUnzip + EntityLimiter combination.
What about Entity.Strict in Http4s 1.x, I'm not sure that this will be a case, since GUnzip adds the Transfer-Encoding: chunked header, so I think such a request should be treated as a streaming one.

Sorry for the delay, I'm a bit stuck figuring out why tests are failing for rootJS and rootNative, while they succeed for JVM. I'll write an update when I unlock this.

I've reworked the gzip bomb test. The test compresses 2 MiB of zeros (which gzip shrinks to ~2 KB) and sets an EntityLimiter of 1 MiB. I believe this is enough to demonstrate the required scenario: the compressed payload is tiny, but decompression produces a body that exceeds the configured limit, resulting in EntityTooLarge. I dropped the approach with gigabytes-scale payload because it was causing CI timeouts on JS/Native.

This is really large, JS/Native are a bit slower, that's why they are timing out and failing.

Good point, thanks! Replaced with a 2 MiB compressed-zeros payload that exercises the same path.