Skip to content

Add option to exclude content types from gzip compression#134

Closed
mmenanno wants to merge 2 commits into
basecamp:mainfrom
mmenanno:gzip-except-content-types
Closed

Add option to exclude content types from gzip compression#134
mmenanno wants to merge 2 commits into
basecamp:mainfrom
mmenanno:gzip-except-content-types

Conversation

@mmenanno

Copy link
Copy Markdown

Thruster gzips everything except gzhttp's built-in exclusions (video/*, audio/*, JPEG). That still compresses image/png, image/webp, image/avif and similar, which are already compressed, so it just burns CPU and forces chunked encoding that drops Content-Length. Right now the only way to avoid that is turning compression off entirely, which also disables it for text.

This adds GZIP_COMPRESSION_EXCEPT_CONTENT_TYPES, a comma-separated list of content types to skip on top of the built-in exclusions. It's matched by prefix, so image/ skips all images while image/png skips only PNG.

The filter extends gzhttp's default rather than replacing it, so leaving the variable unset behaves exactly as before.

GZIP_COMPRESSION_EXCEPT_CONTENT_TYPES=image/png,image/webp,image/avif

mmenanno added 2 commits June 20, 2026 14:44
Add GZIP_COMPRESSION_EXCEPT_CONTENT_TYPES, a comma-separated list of
content types to exclude from gzip compression in addition to gzhttp's
built-in exclusions. Matched by prefix, so "image/" excludes all image
types while "image/png" excludes only PNG.

gzhttp's DefaultContentTypeFilter already skips video/*, audio/*, and
JPEG, but compresses image/png, image/webp, image/avif, etc. Those are
already compressed, so gzipping them wastes CPU and forces chunked
transfer encoding, which drops Content-Length. The only prior workaround
was disabling compression entirely, which also turns it off for text.

The new filter extends the default rather than replacing it, so leaving
the variable unset is byte-identical to current behaviour.
Cover the behaviours introduced by GZIP_COMPRESSION_EXCEPT_CONTENT_TYPES
that the happy-path tests missed: matching a non-first prefix,
case-insensitive matching of a mixed-case configured value, blank
entries being ignored (rather than excluding everything), built-in
exclusions still applying alongside configured ones, and the empty-slice
no-op that the production default relies on.
Copilot AI review requested due to automatic review settings June 20, 2026 19:00

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a configurable way to skip gzip compression for already-compressed response types (e.g., PNG/WebP/AVIF) while preserving gzhttp’s built-in exclusions and keeping default behavior unchanged when the option is unset. This fits cleanly into Thruster’s existing gzip configuration flow by threading a new config field through Service -> HandlerOptions -> CompressionHandler and extending gzhttp’s content-type filter.

Changes:

  • Adds GZIP_COMPRESSION_EXCEPT_CONTENT_TYPES (comma-separated, prefix-matched) to configuration and documentation.
  • Extends the gzip wrapper setup to apply an additional content-type exclusion filter on top of gzhttp.DefaultContentTypeFilter.
  • Adds focused unit tests covering prefix matching, normalization, parameterized content types, and “no-op when unset/empty” behavior.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
README.md Documents the new GZIP_COMPRESSION_EXCEPT_CONTENT_TYPES env var and its prefix-matching behavior.
internal/service.go Passes the new config value into HandlerOptions so it reaches the HTTP pipeline.
internal/handler.go Threads the exception list into NewCompressionHandler.
internal/config.go Parses GZIP_COMPRESSION_EXCEPT_CONTENT_TYPES into Config.GzipCompressionExceptContentTypes.
internal/config_test.go Verifies default/override behavior for the new config field.
internal/compression_handler.go Adds a filter that extends gzhttp.DefaultContentTypeFilter with configurable prefix exclusions.
internal/compression_handler_test.go Adds tests validating the new filtering behavior and ensuring defaults remain unchanged.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@kevinmcconnell

Copy link
Copy Markdown
Collaborator

Thanks for bringing this up @mmenanno! I agree we shouldn't be wasting time compressing these image types. In fact that was a test trying to show that we weren't, but it was only targeting JPEG which seems to be the one image type that gzhttp excludes by default.

Given Thruster's goal of being as nearly-zero-config as possible, I think we should just exclude these types by default rather than introduce new config. I've opened #137 to do that.

If we hit upon a strong argument for needing to override this on a per-app basis we could introduce the config then. But having this in the default behaviour seems like it should be a win for apps in almost all cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants