Streaming gzip response compression

[david-crespo]

Closes #221

• Compression is opt-in: compression config option defaults to false to avoid existing consumers accidentally changing behavior on upgrade
• Compression depends on Accept-Encoding request header as well as the MIME type matching a hard-coded list of compressible types (JSON, text, XML)
• Vary: Accept-Encoding header is added to all compressible responses, which makes sure caches don't, e.g., cache a compressed version and serve it to someone who can't accept it
• Don't bother compressing responses where we know the size and it's less than 512 bytes
• NoCompression extension lets an endpoint opt out of compression

[david-crespo]

This seems like a way for a dropshot handler to tell dropshot not to compress the response even though it might otherwise compress it. That's neat, but happy to get rid of it if we don't need it.

[ahl]

meh up to you

[david-crespo]

I don't see a downside to keeping it so I'll keep it

[david-crespo]

Used for converting between AsyncRead/AsyncWrite from async-compression and the streams used by Body from hyper.

[david-crespo]

This is the primary happy path test for compression.

[ahl]

I gave this a look over and it looks good; let me know if you'd like me to look in more detail. One thing I'm unsure of is how the content-length is set given the streaming compression. It should reflect the compressed size rather than the original size--is that right? Thanks for doing this.

[david-crespo]

I was confused about what is supposed to happen if the client can't handle a streaming response, but it seems all HTTP/1.1 clients must be able to handle chunked transfer encoding:

A recipient MUST be able to parse the chunked transfer coding (Section 4.1) because it plays a crucial role in framing messages when the payload body size is not known in advance.

https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.1

Full answer from GPT-5 through the Codex CLI below. Pretty helpful. The line "HTTP/1.1 clients that advertise Accept-Encoding: gzip are already required to understand chunked responses." is technically true but confusingly phrased because it makes it sound like the requirement is somehow tied to header when it is not.

---

Dropshot only decides to gzip when should_compress_response returns true (dropshot/src/compression.rs:100), which already filters out cases that inherently need fixed-size bodies: no body statuses, HEAD, partial ranges, missing Content-Type, SSE, or content smaller than 512 bytes. Once we do compress, apply_gzip_compression wraps the body in a streaming Gzip encoder and explicitly strips Content-Length/Accept-Ranges (dropshot/src/compression.rs:191 and dropshot/src/compression.rs:238). Hyper will therefore deliver the response with chunked transfer coding on HTTP/1.1, even if the original body was a single buffer. That's the trade-off for not buffering the full compressed entity up front—we never know the final size early enough to set Content-Length.

Why this doesn't break non-streaming clients: HTTP/1.1 clients that advertise Accept-Encoding: gzip are already required to understand chunked responses. Clients that cannot handle streaming should omit Accept-Encoding (they'll stay on the uncompressed path with a fixed length) or explicitly disable compression via the NoCompression extension. Our streaming endpoints continue to stream but remain uncompressed because they typically lack a compressible Content-Type, which should_compress_response uses to short-circuit (dropshot/tests/integration-tests/gzip.rs:344). Tests cover all combinations: buffered responses keep a fixed length (dropshot/tests/integration-tests/streaming.rs:55), streaming endpoints stay chunked (dropshot/tests/integration-tests/streaming.rs:91), and streaming + gzip gets rejected (dropshot/tests/integration-tests/gzip.rs:366).

Matrix (server view vs client capability):

• Server sends fixed-length body → clients that can't stream are fine; clients that can stream are also fine.
• Server sends chunked/streaming body (including gzip) → clients that can stream are fine; clients that can't must not have asked for compression, so they fall back to the first row.

If you have a population of Accept-Encoding: gzip clients that truly cannot process chunked bodies, the short-term mitigation is to strip or downgrade that header in front of Dropshot; longer term you could add an alternate compression path that buffers and sets Content-Length, but that's intentionally avoided in the current design.

[ahl]

Thanks for the clarification. I see in the tests how you're validating the content-length header, etc.

Should compression be configurable in the server settings? i.e. would it make sense to e.g. have compression=on be the default but allow one to make a server that never compresses responses?

[david-crespo]

Honestly not sure. Kind of hard to imagine why you would want to not compress if a client asked for it. But on the other hand it seems heavy-handed to just do it.

[david-crespo]

I added the config option (default true) and I used some helpers to shorten the tests a bit, which to me makes them a little more scannable, though you might prefer things inlined. I think this is ready for a real review.

[david-crespo]

From the commit message on 9be5395:

Gemini's theory: When a test case requests the /large-response
endpoint but compression is disabled (either by config, lack of header,
or rejection), the server attempts to write the full ~5KB JSON body to
the socket. On illumos, the default TCP socket buffer size is likely
smaller than on macOS/Linux, causing the server's write operation to
block because the test client never reads the data to drain the buffer.
When teardown() is called, the server hangs trying to finish the write
during graceful shutdown, eventually timing out.

The fix is to consume the response body in these tests.

The fix did work, so it seems directionally right even if the TCP socket buffer size isn't the precise reason. Not sure whether it's worth chasing down the reason further.

[david-crespo]

It does not fail on macOS with a smaller TCP socket buffer (thanks @rmustacc for the suggestion):

$ sysctl net.inet.tcp.sendspace net.inet.tcp.recvspace
net.inet.tcp.sendspace: 131072
net.inet.tcp.recvspace: 131072

$ sudo sysctl net.inet.tcp.sendspace=4096
$ sudo sysctl net.inet.tcp.recvspace=4096

Will test it on illumos. Could not repro the failure in illumos on atrium. All tests passed. The buffer sizes are plenty big. I don't have permission to change them, will look into it.

dcrespo@atrium ~/dropshot $ ipadm show-prop -p send_buf tcp
PROTO PROPERTY              PERM CURRENT      PERSISTENT   DEFAULT      POSSIBLE
tcp   send_buf              rw   256000       256000       49152        4096-1048576
dcrespo@atrium ~/dropshot $ ipadm show-prop -p recv_buf tcp
PROTO PROPERTY              PERM CURRENT      PERSISTENT   DEFAULT      POSSIBLE
tcp   recv_buf              rw   512000       512000       128000       2048-1048576

[ahl]

This is looking good; can you please update the changelog to document this?

[ahl]

Are there more than 2 options when it comes to compression? I.e. might there be additional configuration in the future; rather than a bool might we want an enum? i.e. avoid further breaking changes

[david-crespo]

Very good point. I checked what tower-http do and they have a lot more than a bool going on:

pub enum CompressionLevel {
    Fastest,
    Best,
    #[default]
    Default,
    Precise(i32),
}

pub(crate) struct AcceptEncoding {
    pub(crate) gzip: bool,
    pub(crate) deflate: bool,
    pub(crate) br: bool,
    pub(crate) zstd: bool,
}

pub struct CompressionLayer<P = DefaultPredicate> {
    accept: AcceptEncoding,
    predicate: P,
    quality: CompressionLevel,
}

That is overkill for us but it gives a sense of the way we might complicate this. The most likely one to me would be that we add brotli. I'll figure it out.

https://github.com/tower-rs/tower-http/blob/3bf1ba7b7893b57264dfe663165a2bc57a40d2c4/tower-http/src/compression/layer.rs#L7-L18

https://github.com/tower-rs/tower-http/blob/3bf1ba7b7893b57264dfe663165a2bc57a40d2c4/tower-http/src/compression_utils.rs#L425-L445

https://github.com/tower-rs/tower-http/blob/3bf1ba7b7893b57264dfe663165a2bc57a40d2c4/tower-http/src/compression_utils.rs#L18-L23

[ahl]

meh up to you

[david-crespo]

Tested in Nexus locally (pushed changes as test-gzip branch, required cargo update -p hyper -p slog -p camino -p toml@0.9.5 -p hyper-util -p serde_path_to_error to compile). It seems to work. Confirmed 80% reduction in size of project list response after creating 20 projects.