Don't compress a response whose handler already set Content-Encoding - #2575
Conversation
The guard that stands down when a response already names a content coding covered the responses that settle their coding in `apply_ranges()`, but a file-backed one settles it in `static_file_encoding()`, which asked the content-type overload and so never saw the field. With static file compression enabled, a mount point naming the coding for a tree of build-time compressed assets, and a handler setting the field on a `set_file_content()` response, both had their stored bytes compressed a second time and a second `Content-Encoding` field line appended. A file-backed response has not been given a content type by the time its coding is decided, which is the only reason it could not go through `encoding_type()`. It takes the type as an argument now, so both paths share the one guard instead of carrying a copy each. `Response::content_encoding_` becomes `content_coding_`, after what it holds. It names the coding chosen for the body, which is what its own comment already called it, while the old name read as the value of the `Content-Encoding` field whose presence is exactly what forces the coding to `None`. README gains the behaviour, including the part that stays with the handler: `Vary` is added only to a coding the server chose, so a handler that picks a representation from `Accept-Encoding` has to add the field itself.
|
Thanks again for the fix, I've merged it. While reviewing I noticed static file responses had the same issue this PR fixes elsewhere: I pushed a small follow-up commit onto this branch to close that gap. It shares your guard rather than duplicating it: Hope that's a welcome addition rather than a surprise — happy to adjust anything if you'd rather it looked different. |
2 participants
A handler serving pre-compressed content (e.g. build-time gzipped static assets) sets
Content-Encodingitself. The server only consultsAccept-Encodingand the content type, so it compresses the body a second time and appends a secondContent-Encoding: gzipfield line, clients that decode one coding per listed value (like Safari) render raw gzip bytes. The test suite currently works around it ("image/jpegkeeps the server from applying a content coding of its own"). Hit by llama.cpp's server while enabling zlib support (ggml-org/llama.cpp#28059).Fix:
detail::encoding_type(req, res)returnsNonewhen the response already carriesContent-Encoding. Since the chunked path used to negotiate twice (headers inapply_ranges(), compressor at write time: where theguard would now misfire on the header
apply_ranges()itself set), the coding is decided once:file_content_encoding_(#2572) is generalized tocontent_encoding_and shared by the file-backed and chunked provider paths, which are mutually exclusive.Adds
ContentEncodingTest.PreEncodedResponseIsNotCompressedAgain(fails before this change: two field lines, double-encoded body). Full suite passes with ZLIB, Brotli, (887 tests).NOTE: The fix is helped by Claude Code.