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

Check Content-Length against minSize and passthrough writes if no gzip #71

Closed jameshartig closed 6 years ago

jameshartig commented 6 years ago

Currently there's no way to generate a HEAD response with the correct headers as the GET unless you set the minSize as 0, but then gzip headers will be written in Close. Instead, allow a Write(nil) that will set the correct headers based on the Content-Length/Content-Type headers and only initialize a writer if there is a non-zero-length Write. If the Content-Length cannot be determined, you cannot generate the response because it cannot know if minSize would've been met.

Fixes #70

jameshartig commented 6 years ago

I also handled the issue mentioned in #64.

The benchmarks seem to show the BenchmarkGzipHandler_S2k-8 got faster while the BenchmarkGzipHandler_P20k-8 and BenchmarkGzipHandler_P100k-8 got slower.

Before:
BenchmarkGzipHandler_S2k-8           200       8180495 ns/op
BenchmarkGzipHandler_S20k-8          100      24256560 ns/op
BenchmarkGzipHandler_S100k-8          20     100509175 ns/op
BenchmarkGzipHandler_P2k-8          1000       1766909 ns/op
BenchmarkGzipHandler_P20k-8          200       5878452 ns/op
BenchmarkGzipHandler_P100k-8          50      26159506 ns/op

After:
BenchmarkGzipHandler_S2k-8           200       6932798 ns/op
BenchmarkGzipHandler_S20k-8          100      24246411 ns/op
BenchmarkGzipHandler_S100k-8          20     101508020 ns/op
BenchmarkGzipHandler_P2k-8          1000       1691251 ns/op
BenchmarkGzipHandler_P20k-8          200       7326576 ns/op
BenchmarkGzipHandler_P100k-8          30      33444100 ns/op
jameshartig commented 6 years ago

@jprobinson Not sure if I should @ you or if you get notifications from PRs. Thanks!

jprobinson commented 6 years ago

@fastest963 Hi! Sorry, I've been a bit busy. I've seen this but will need to find some time to review the PR. I'll try and get to it soon.

jameshartig commented 6 years ago

@jprobinson okay, thanks! No problem, just wanted to check on it!

jprobinson commented 6 years ago

thanks again, @fastest963!

jameshartig commented 6 years ago

No problem! Thanks for merging!