From fee0b38b4842403a28d664385a1ff1075e8891d2 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Sat, 29 Jun 2019 16:57:55 -0600 Subject: [PATCH] Fix encoder name bug; remove unused field in encode middleware struct --- modules/caddyhttp/encode/brotli/brotli.go | 6 +++- modules/caddyhttp/encode/encode.go | 36 +++++++++++------------ modules/caddyhttp/encode/gzip/gzip.go | 6 +++- modules/caddyhttp/encode/zstd/zstd.go | 4 +++ 4 files changed, 31 insertions(+), 21 deletions(-) diff --git a/modules/caddyhttp/encode/brotli/brotli.go b/modules/caddyhttp/encode/brotli/brotli.go index 6bfd11f7..dc4559e8 100644 --- a/modules/caddyhttp/encode/brotli/brotli.go +++ b/modules/caddyhttp/encode/brotli/brotli.go @@ -35,6 +35,10 @@ func (b Brotli) Validate() error { return nil } +// AcceptEncoding returns the name of the encoding as +// used in the Accept-Encoding request headers. +func (Brotli) AcceptEncoding() string { return "br" } + // NewEncoder returns a new brotli writer. func (b Brotli) NewEncoder() encode.Encoder { quality := brotli.DefaultCompression @@ -46,6 +50,6 @@ func (b Brotli) NewEncoder() encode.Encoder { // Interface guards var ( - _ encode.Encoding = (*Brotli)(nil) + _ encode.Encoding = (*Brotli)(nil) _ caddy.Validator = (*Brotli)(nil) ) diff --git a/modules/caddyhttp/encode/encode.go b/modules/caddyhttp/encode/encode.go index 79294052..8d8aaf37 100644 --- a/modules/caddyhttp/encode/encode.go +++ b/modules/caddyhttp/encode/encode.go @@ -1,8 +1,8 @@ // Package encode implements an encoder middleware for Caddy. The initial // enhancements related to Accept-Encoding, minimum content length, and // buffer/writer pools were adapted from https://github.com/xi2/httpgzip -// then modified heavily to accommodate modular encoders. Code borrowed -// from that repository is Copyright (c) 2015 The Httpgzip Authors. +// then modified heavily to accommodate modular encoders and fix bugs. +// Code borrowed from that repository is Copyright (c) 2015 The Httpgzip Authors. package encode import ( @@ -33,14 +33,11 @@ type Encode struct { Prefer []string `json:"prefer,omitempty"` MinLength int `json:"minimum_length,omitempty"` - Encodings map[string]Encoding `json:"-"` - writerPools map[string]*sync.Pool // TODO: these pools do not get reused through config reloads... } // Provision provisions enc. func (enc *Encode) Provision(ctx caddy.Context) error { - enc.Encodings = make(map[string]Encoding) enc.writerPools = make(map[string]*sync.Pool) for modName, rawMsg := range enc.EncodingsRaw { @@ -49,9 +46,8 @@ func (enc *Encode) Provision(ctx caddy.Context) error { return fmt.Errorf("loading encoder module '%s': %v", modName, err) } encoder := val.(Encoding) - enc.Encodings[modName] = encoder - enc.writerPools[modName] = &sync.Pool{ + enc.writerPools[encoder.AcceptEncoding()] = &sync.Pool{ New: func() interface{} { return encoder.NewEncoder() }, @@ -159,17 +155,6 @@ func (rw *responseWriter) Write(p []byte) (int, error) { return n, err } -// init should be called before we write a response, if rw.buf has contents. -func (rw *responseWriter) init() { - if rw.Header().Get("Content-Encoding") == "" && rw.buf.Len() >= rw.config.MinLength { - rw.w = rw.config.writerPools[rw.encodingName].Get().(Encoder) - rw.w.Reset(rw.ResponseWriter) - rw.Header().Del("Content-Length") // https://github.com/golang/go/issues/14975 - rw.Header().Set("Content-Encoding", rw.encodingName) - } - rw.Header().Del("Accept-Ranges") // we don't know ranges for dynamically-encoded content -} - // Close writes any remaining buffered response and // deallocates any active resources. func (rw *responseWriter) Close() error { @@ -213,6 +198,17 @@ func (rw *responseWriter) Close() error { return err } +// init should be called before we write a response, if rw.buf has contents. +func (rw *responseWriter) init() { + if rw.Header().Get("Content-Encoding") == "" && rw.buf.Len() >= rw.config.MinLength { + rw.w = rw.config.writerPools[rw.encodingName].Get().(Encoder) + rw.w.Reset(rw.ResponseWriter) + rw.Header().Del("Content-Length") // https://github.com/golang/go/issues/14975 + rw.Header().Set("Content-Encoding", rw.encodingName) + } + rw.Header().Del("Accept-Ranges") // we don't know ranges for dynamically-encoded content +} + // acceptedEncodings returns the list of encodings that the // client supports, in descending order of preference. If // the Sec-WebSocket-Key header is present then non-identity @@ -288,8 +284,10 @@ type Encoder interface { Reset(io.Writer) } -// Encoding is a type which can create encoders of its kind. +// Encoding is a type which can create encoders of its kind +// and return the name used in the Accept-Encoding header. type Encoding interface { + AcceptEncoding() string NewEncoder() Encoder } diff --git a/modules/caddyhttp/encode/gzip/gzip.go b/modules/caddyhttp/encode/gzip/gzip.go index 156eb0d2..f8e16c9f 100644 --- a/modules/caddyhttp/encode/gzip/gzip.go +++ b/modules/caddyhttp/encode/gzip/gzip.go @@ -40,6 +40,10 @@ func (g Gzip) Validate() error { return nil } +// AcceptEncoding returns the name of the encoding as +// used in the Accept-Encoding request headers. +func (Gzip) AcceptEncoding() string { return "gzip" } + // NewEncoder returns a new gzip writer. func (g Gzip) NewEncoder() encode.Encoder { writer, _ := gzip.NewWriterLevel(nil, g.Level) @@ -51,7 +55,7 @@ var defaultGzipLevel = 5 // Interface guards var ( - _ encode.Encoding = (*Gzip)(nil) + _ encode.Encoding = (*Gzip)(nil) _ caddy.Provisioner = (*Gzip)(nil) _ caddy.Validator = (*Gzip)(nil) ) diff --git a/modules/caddyhttp/encode/zstd/zstd.go b/modules/caddyhttp/encode/zstd/zstd.go index 2bb8771e..dbcc3ec1 100644 --- a/modules/caddyhttp/encode/zstd/zstd.go +++ b/modules/caddyhttp/encode/zstd/zstd.go @@ -16,6 +16,10 @@ func init() { // Zstd can create zstd encoders. type Zstd struct{} +// AcceptEncoding returns the name of the encoding as +// used in the Accept-Encoding request headers. +func (Zstd) AcceptEncoding() string { return "zstd" } + // NewEncoder returns a new gzip writer. func (z Zstd) NewEncoder() encode.Encoder { writer, _ := zstd.NewWriter(nil)