Skip to content

Commit

Permalink
Revert "Add support to skiping gzip for small responses. Fixes nytime…
Browse files Browse the repository at this point in the history
…s#24 (nytimes#30)" (nytimes#33)

This reverts commit bdb2763.
  • Loading branch information
adammck authored and jprobinson committed Mar 7, 2017
1 parent bdb2763 commit fb35337
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 79 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
language: go

go:
- 1.x
- 1.7
- tip
38 changes: 7 additions & 31 deletions gzip.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,16 @@ const (

type codings map[string]float64

const (
// DefaultQValue is the default qvalue to assign to an encoding if no explicit qvalue is set.
// This is actually kind of ambiguous in RFC 2616, so hopefully it's correct.
// The examples seem to indicate that it is.
DefaultQValue = 1.0

// DefaultMinSize defines the minimum size to reach to enable compression.
// It's 512 bytes.
DefaultMinSize = 512
)
// The default qvalue to assign to an encoding if no explicit qvalue is set.
// This is actually kind of ambiguous in RFC 2616, so hopefully it's correct.
// The examples seem to indicate that it is.
const DEFAULT_QVALUE = 1.0

// gzipWriterPools stores a sync.Pool for each compression level for reuse of
// gzip.Writers. Use poolIndex to covert a compression level to an index into
// gzipWriterPools.
var gzipWriterPools [gzip.BestCompression - gzip.BestSpeed + 2]*sync.Pool

// minSize gives the hability to skeep compression on response smaller than the
// given size. The default value is 512 bytes.

func init() {
for i := gzip.BestSpeed; i <= gzip.BestCompression; i++ {
addLevelPool(i)
Expand Down Expand Up @@ -74,17 +65,12 @@ func addLevelPool(level int) {
// writers, so don't forget to do that.
type GzipResponseWriter struct {
http.ResponseWriter
index int // Index for gzipWriterPools.
gw *gzip.Writer
minSize int
index int // Index for gzipWriterPools.
gw *gzip.Writer
}

// Write appends data to the gzip writer.
func (w *GzipResponseWriter) Write(b []byte) (int, error) {
if len(b) < w.minSize {
return w.ResponseWriter.Write(b)
}

// Lazily create the gzip.Writer, this allows empty bodies to be actually
// empty, for example in the case of status code 204 (no content).
if w.gw == nil {
Expand Down Expand Up @@ -176,18 +162,9 @@ func MustNewGzipLevelHandler(level int) func(http.Handler) http.Handler {
// if an invalid gzip compression level is given, so if one can ensure the level
// is valid, the returned error can be safely ignored.
func NewGzipLevelHandler(level int) (func(http.Handler) http.Handler, error) {
return NewGzipLevelAndMinSize(level, DefaultMinSize)
}

// NewGzipLevelAndMinSize do as above but let caller specify the minimum size
// before compression
func NewGzipLevelAndMinSize(level, askedMinSize int) (func(http.Handler) http.Handler, error) {
if level != gzip.DefaultCompression && (level < gzip.BestSpeed || level > gzip.BestCompression) {
return nil, fmt.Errorf("invalid compression level requested: %d", level)
}
if askedMinSize < 0 {
return nil, fmt.Errorf("minimum size must be more than zero")
}
return func(h http.Handler) http.Handler {
index := poolIndex(level)

Expand All @@ -198,7 +175,6 @@ func NewGzipLevelAndMinSize(level, askedMinSize int) (func(http.Handler) http.Ha
gw := &GzipResponseWriter{
ResponseWriter: w,
index: index,
minSize: askedMinSize,
}
defer gw.Close()

Expand Down Expand Up @@ -261,7 +237,7 @@ func parseEncodings(s string) (codings, error) {
func parseCoding(s string) (coding string, qvalue float64, err error) {
for n, part := range strings.Split(s, ";") {
part = strings.TrimSpace(part)
qvalue = DefaultQValue
qvalue = DEFAULT_QVALUE

if n == 0 {
coding = strings.ToLower(part)
Expand Down
53 changes: 6 additions & 47 deletions gzip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ import (
"github.com/stretchr/testify/assert"
)

const testBody = "aaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbcccaaabbbccc"

func TestParseEncodings(t *testing.T) {

examples := map[string]codings{

// Examples from RFC 2616
Expand All @@ -40,6 +39,8 @@ func TestParseEncodings(t *testing.T) {
}

func TestGzipHandler(t *testing.T) {
testBody := "aaabbbccc"

// This just exists to provide something for GzipHandler to wrap.
handler := newTestHandler(testBody)

Expand Down Expand Up @@ -79,6 +80,7 @@ func TestGzipHandler(t *testing.T) {
}

func TestNewGzipLevelHandler(t *testing.T) {
testBody := "aaabbbccc"
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
io.WriteString(w, testBody)
Expand All @@ -100,6 +102,7 @@ func TestNewGzipLevelHandler(t *testing.T) {
assert.Equal(t, "gzip", res.Header.Get("Content-Encoding"))
assert.Equal(t, "Accept-Encoding", res.Header.Get("Vary"))
assert.Equal(t, gzipStrLevel(testBody, lvl), resp.Body.Bytes())

}
}

Expand Down Expand Up @@ -167,7 +170,7 @@ func TestGzipHandlerNoBody(t *testing.T) {
}

func TestGzipHandlerContentLength(t *testing.T) {
b := []byte(testBody)
b := []byte("testtesttesttesttesttesttesttesttesttesttesttesttest")
handler := GzipHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Length", strconv.Itoa(len(b)))
w.Write(b)
Expand Down Expand Up @@ -212,50 +215,6 @@ func TestGzipHandlerContentLength(t *testing.T) {
assert.NotEqual(t, b, body)
}

func TestGzipHandlerMinSize(t *testing.T) {
// Run a test with size smaller than the limit
b := bytes.NewBufferString("test")

wrapper, _ := NewGzipLevelAndMinSize(gzip.DefaultCompression, 12)
handler := wrapper(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
resp, _ := ioutil.ReadAll(r.Body)
w.Write(resp)
},
))

req1, _ := http.NewRequest("GET", "/whatever", b)
req1.Header.Add("Accept-Encoding", "gzip")
resp1 := httptest.NewRecorder()
handler.ServeHTTP(resp1, req1)
res1 := resp1.Result()

if res1.Header.Get(contentEncoding) == "gzip" {
t.Errorf("The response is compress and should not")
return
}

// Run a test with size bigger than the limit
b = bytes.NewBufferString(testBody)

req2, _ := http.NewRequest("GET", "/whatever", b)
req2.Header.Add("Accept-Encoding", "gzip")
resp2 := httptest.NewRecorder()
handler.ServeHTTP(resp2, req2)
res2 := resp2.Result()

if res2.Header.Get(contentEncoding) != "gzip" {
t.Errorf("The response is not compress and should")
return
}

_, errorMinNegative := NewGzipLevelAndMinSize(gzip.DefaultCompression, -10)
if errorMinNegative == nil {
t.Error("The minimum size it negative and the function returns no error")
return
}
}

// --------------------------------------------------------------------

func BenchmarkGzipHandler_S2k(b *testing.B) { benchmark(b, false, 2048) }
Expand Down

0 comments on commit fb35337

Please sign in to comment.