fix: skip gzip for compressed types, parse Accept-Encoding quality values#6
Draft
troglodyne-bot wants to merge 1 commit into
Draft
Conversation
…lues Two bugs in HTTP compression handling: 1. Accept-Encoding parsing used string equality on the raw token, so 'gzip;q=0.9' never matched 'gzip'. Strip quality params before comparing. Extracted into _accepts_gzip() for testability. 2. serve() gzip'd all files regardless of MIME type — compressing already-compressed formats (JPEG, PNG, ZIP, PDF, etc.) wastes CPU and produces larger responses. Added %incompressible_mime guard. Also removes duplicate 'use File::Find' import. Tests: t/04-compression.t — 14 assertions covering both bugs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
teodesian
reviewed
Jun 3, 2026
|
|
||
| my $ct = 'Content-type'; | ||
|
|
||
| # MIME types that are already compressed — gzip would waste CPU and bloat responses |
Contributor
There was a problem hiding this comment.
this is a less than comprehensive list. I'd prefer if it was accurate for at least the list of mime types understood by Plack::MIME, given that's what we're using in practice to figure mimetypes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two bugs in HTTP compression handling fixed, with tests.
Why
Accept-Encoding quality values silently disabled gzip:
'gzip;q=0.9'never matched'gzip'via equality — any client that sends quality parameters (proxies, HTTP/2 negotiators) got uncompressed responses without any error.Already-compressed MIME types were gzip'd anyway: PNG, JPEG, ZIP, PDF, WebP, and a dozen other pre-compressed formats were run through
IO::Compress::Gzipevery request. These formats don't compress further — they only get larger and waste CPU. On a media-heavy site this is pure overhead.Also removes a duplicate
use File::Findimport (lines 27 and 36 pre-patch).How
_accepts_gzip($header)— strips quality params (s/;.*//r) before matching, returns explicit1/0.%incompressible_mime— a compile-time hash of MIME types that should never be gzip'd (images, audio, video, archives, PDF, WASM, Office docs).serve()now checks$incompressible_mime{$ft}before compressing; skips to the uncompressed path if matched.Testing
t/04-compression.t— 14 assertions:_accepts_gzip: covers plain, quality values, multiple encodings, false casesserve()compression behavior: text gets encoded, PNG/JPEG/ZIP do not,deflate=0disables compression