search

nytimes / gziphandler

Go middleware to gzip HTTP responses
https://godoc.org/github.com/NYTimes/gziphandler
Apache License 2.0
868 stars 129 forks source link

Add NotContentTypes option #81

Open jared2501 opened 5 years ago

jared2501 commented 5 years ago

Use case: I would like to gzip all content-types, except images/videos.

jared2501 commented 5 years ago

hey @fsouza, @jprobinson - would ya'll be interested in this change? If so, happy to add tests / refactor.

jprobinson commented 5 years ago

@jared2501 this seems like a reasonable addition to me. The code looks like it's in a good place. Go ahead and add some tests and I'll bring this in. Thanks!

fsouza commented 5 years ago

oh sorry I missed this. Yeah I second @jp's comment and I was actually thinking about this over the weekend (we have a gizmo service that generates video thumbnails and we don't want to gzip jpg images :D).

adammck commented 5 years ago

Hi! Because I like functional options so much, I might personally implement this api as:

interface contentTypeMatcher {
  Matches(contentType string) bool
}

func ContentTypeMatchers(matchers ... contentTypeMatcher) option {
   // ...
}

gzip.GzipHandlerWithOpts(gzip.ContentTypeMatchers(
  gzip.MatchLiteral("text/plain"),
  gzip.MatchLiteral("image/jpg", invert=True), // does go have named params? i forgot
))

...so that in some magical future, you might do such weird things as:

gzip.MatchPrefix("text/")
gzip.MatchRegex("*/*+json")
gzip.MatchFunc(func(ct string) bool {
  return time.Now().Hour < 12
})

but then I'd probably add a helper that looks just like what you're proposing, so I dunno.

mkraft commented 4 years ago

I have opened a PR for this also: https://github.com/nytimes/gziphandler/pull/98