From df142654d68c15ea8a1efa841575c9b08910d7e0 Mon Sep 17 00:00:00 2001 From: Jim Kalafut Date: Thu, 12 Sep 2019 22:32:22 -0700 Subject: [PATCH 1/3] Fix Agent handling of gzipped responses Fixes #6606 --- command/agent/cache/api_proxy.go | 38 +++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/command/agent/cache/api_proxy.go b/command/agent/cache/api_proxy.go index f0a919121a0a..181df97a932e 100644 --- a/command/agent/cache/api_proxy.go +++ b/command/agent/cache/api_proxy.go @@ -3,6 +3,7 @@ package cache import ( "context" "fmt" + "net/http" hclog "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/api" @@ -36,7 +37,16 @@ func (ap *APIProxy) Send(ctx context.Context, req *SendRequest) (*SendResponse, return nil, err } client.SetToken(req.Token) - client.SetHeaders(req.Request.Header) + + // http.Transport will transparently request gzip and decompress the response, but only if + // the client doesn't manually set the header (in which case the client has to handle + // the decompression. If gzip has already been set, remove it to avoid triggering the manual + // handling requirement. + h := clone(req.Request.Header) + if h.Get("Accept-Encoding") == "gzip" { + h.Del("Accept-Encoding") + } + client.SetHeaders(h) fwReq := client.NewRequest(req.Request.Method, req.Request.URL.Path) fwReq.BodyBytes = req.RequestBody @@ -65,3 +75,29 @@ func (ap *APIProxy) Send(ctx context.Context, req *SendRequest) (*SendResponse, // Bubble back the api.Response as well for error checking/handling at the handler layer. return sendResponse, err } + +// clone returns a copy of h or nil if h is nil. +// +// TODO: This is a copy of the new (Header) Clone method in Go 1.13. We should remove this once we update. +func clone(h http.Header) http.Header { + if h == nil { + return nil + } + + // Find total number of values. + nv := 0 + for _, vv := range h { + nv += len(vv) + } + sv := make([]string, nv) // shared backing array for headers' values + h2 := make(http.Header, len(h)) + + for k, vv := range h { + n := copy(sv, vv) + h2[k] = sv[:n:n] + sv = sv[n:] + + } + + return h2 +} From 866bd9c1baf71871a98ac9b59e1b3bd082c91784 Mon Sep 17 00:00:00 2001 From: Jim Kalafut Date: Fri, 13 Sep 2019 08:43:32 -0700 Subject: [PATCH 2/3] Only remove "gzip" member, if present --- command/agent/cache/api_proxy.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/command/agent/cache/api_proxy.go b/command/agent/cache/api_proxy.go index 181df97a932e..3b8d62018b80 100644 --- a/command/agent/cache/api_proxy.go +++ b/command/agent/cache/api_proxy.go @@ -7,6 +7,7 @@ import ( hclog "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/sdk/helper/strutil" ) // APIProxy is an implementation of the proxier interface that is used to @@ -43,8 +44,8 @@ func (ap *APIProxy) Send(ctx context.Context, req *SendRequest) (*SendResponse, // the decompression. If gzip has already been set, remove it to avoid triggering the manual // handling requirement. h := clone(req.Request.Header) - if h.Get("Accept-Encoding") == "gzip" { - h.Del("Accept-Encoding") + if v, ok := h["Accept-Encoding"]; ok { + h["Accept-Encoding"] = strutil.StrListDelete(v, "gzip") } client.SetHeaders(h) From b095e7a10e0bd346ee19b48da50e2eb5b4a721b6 Mon Sep 17 00:00:00 2001 From: Jim Kalafut Date: Fri, 13 Sep 2019 14:44:51 -0700 Subject: [PATCH 3/3] Simplify to just removing Accept-Encoding altogether --- command/agent/cache/api_proxy.go | 40 ++++---------------------------- 1 file changed, 4 insertions(+), 36 deletions(-) diff --git a/command/agent/cache/api_proxy.go b/command/agent/cache/api_proxy.go index 3b8d62018b80..580c14f75f2c 100644 --- a/command/agent/cache/api_proxy.go +++ b/command/agent/cache/api_proxy.go @@ -3,11 +3,9 @@ package cache import ( "context" "fmt" - "net/http" hclog "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/api" - "github.com/hashicorp/vault/sdk/helper/strutil" ) // APIProxy is an implementation of the proxier interface that is used to @@ -40,14 +38,10 @@ func (ap *APIProxy) Send(ctx context.Context, req *SendRequest) (*SendResponse, client.SetToken(req.Token) // http.Transport will transparently request gzip and decompress the response, but only if - // the client doesn't manually set the header (in which case the client has to handle - // the decompression. If gzip has already been set, remove it to avoid triggering the manual - // handling requirement. - h := clone(req.Request.Header) - if v, ok := h["Accept-Encoding"]; ok { - h["Accept-Encoding"] = strutil.StrListDelete(v, "gzip") - } - client.SetHeaders(h) + // the client doesn't manually set the header. Removing any Accept-Encoding header allows the + // transparent compression to occur. + req.Request.Header.Del("Accept-Encoding") + client.SetHeaders(req.Request.Header) fwReq := client.NewRequest(req.Request.Method, req.Request.URL.Path) fwReq.BodyBytes = req.RequestBody @@ -76,29 +70,3 @@ func (ap *APIProxy) Send(ctx context.Context, req *SendRequest) (*SendResponse, // Bubble back the api.Response as well for error checking/handling at the handler layer. return sendResponse, err } - -// clone returns a copy of h or nil if h is nil. -// -// TODO: This is a copy of the new (Header) Clone method in Go 1.13. We should remove this once we update. -func clone(h http.Header) http.Header { - if h == nil { - return nil - } - - // Find total number of values. - nv := 0 - for _, vv := range h { - nv += len(vv) - } - sv := make([]string, nv) // shared backing array for headers' values - h2 := make(http.Header, len(h)) - - for k, vv := range h { - n := copy(sv, vv) - h2[k] = sv[:n:n] - sv = sv[n:] - - } - - return h2 -}