diff --git a/.changelog/11843.txt b/.changelog/11843.txt new file mode 100644 index 000000000000..1d62d1a61a25 --- /dev/null +++ b/.changelog/11843.txt @@ -0,0 +1,3 @@ +```release-note:improvement +deps: use gorilla package for gzip http handler +``` diff --git a/api/api.go b/api/api.go index 8719cab2c395..d8b90cc8dc8a 100644 --- a/api/api.go +++ b/api/api.go @@ -741,33 +741,45 @@ func (c *Client) doRequest(r *request) (time.Duration, *http.Response, error) { if err != nil { return 0, nil, err } + start := time.Now() resp, err := c.httpClient.Do(req) diff := time.Since(start) // If the response is compressed, we swap the body's reader. - if resp != nil && resp.Header != nil { - var reader io.ReadCloser - switch resp.Header.Get("Content-Encoding") { - case "gzip": - greader, err := gzip.NewReader(resp.Body) - if err != nil { - return 0, nil, err - } + if zipErr := c.autoUnzip(resp); zipErr != nil { + return 0, nil, zipErr + } - // The gzip reader doesn't close the wrapped reader so we use - // multiCloser. - reader = &multiCloser{ - reader: greader, - inorderClose: []io.Closer{greader, resp.Body}, - } - default: - reader = resp.Body + return diff, resp, err +} + +// autoUnzip modifies resp in-place, wrapping the response body with a gzip +// reader if the Content-Encoding of the response is "gzip". +func (*Client) autoUnzip(resp *http.Response) error { + if resp == nil || resp.Header == nil { + return nil + } + + if resp.Header.Get("Content-Encoding") == "gzip" { + zReader, err := gzip.NewReader(resp.Body) + if err == io.EOF { + // zero length response, do not wrap + return nil + } else if err != nil { + // some other error (e.g. corrupt) + return err + } + + // The gzip reader does not close an underlying reader, so use a + // multiCloser to make sure response body does get closed. + resp.Body = &multiCloser{ + reader: zReader, + inorderClose: []io.Closer{zReader, resp.Body}, } - resp.Body = reader } - return diff, resp, err + return nil } // rawQuery makes a GET request to the specified endpoint but returns just the diff --git a/api/api_test.go b/api/api_test.go index a845f6410547..3f83e716c930 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -1,10 +1,13 @@ package api import ( + "bytes" + "compress/gzip" "context" "encoding/json" "errors" "fmt" + "io" "net/http" "net/http/httptest" "net/url" @@ -568,3 +571,49 @@ func TestClient_HeaderRaceCondition(t *testing.T) { require.Equal(2, <-c, "goroutine request should have two headers") require.Len(conf.Headers, 1, "config headers should not mutate") } + +func TestClient_autoUnzip(t *testing.T) { + var client *Client = nil + + try := func(resp *http.Response, exp error) { + err := client.autoUnzip(resp) + require.Equal(t, exp, err) + } + + // response object is nil + try(nil, nil) + + // response.Body is nil + try(new(http.Response), nil) + + // content-encoding is not gzip + try(&http.Response{ + Header: http.Header{"Content-Encoding": []string{"text"}}, + }, nil) + + // content-encoding is gzip but body is empty + try(&http.Response{ + Header: http.Header{"Content-Encoding": []string{"gzip"}}, + Body: io.NopCloser(bytes.NewBuffer([]byte{})), + }, nil) + + // content-encoding is gzip but body is invalid gzip + try(&http.Response{ + Header: http.Header{"Content-Encoding": []string{"gzip"}}, + Body: io.NopCloser(bytes.NewBuffer([]byte("not a zip"))), + }, errors.New("unexpected EOF")) + + // sample gzip payload + var b bytes.Buffer + w := gzip.NewWriter(&b) + _, err := w.Write([]byte("hello world")) + require.NoError(t, err) + err = w.Close() + require.NoError(t, err) + + // content-encoding is gzip and body is gzip data + try(&http.Response{ + Header: http.Header{"Content-Encoding": []string{"gzip"}}, + Body: io.NopCloser(&b), + }, nil) +} diff --git a/command/agent/http.go b/command/agent/http.go index a3c082661e99..d33be8e85c9a 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -14,8 +14,8 @@ import ( "strings" "time" - "github.com/NYTimes/gziphandler" assetfs "github.com/elazarl/go-bindata-assetfs" + "github.com/gorilla/handlers" "github.com/gorilla/websocket" "github.com/hashicorp/go-connlimit" log "github.com/hashicorp/go-hclog" @@ -86,12 +86,6 @@ func NewHTTPServers(agent *Agent, config *Config) ([]*HTTPServer, error) { var srvs []*HTTPServer var serverInitializationErrors error - // Handle requests with gzip compression - gzip, err := gziphandler.GzipHandlerWithOpts(gziphandler.MinSize(0)) - if err != nil { - return srvs, err - } - // Get connection handshake timeout limit handshakeTimeout, err := time.ParseDuration(config.Limits.HTTPSHandshakeTimeout) if err != nil { @@ -160,7 +154,7 @@ func NewHTTPServers(agent *Agent, config *Config) ([]*HTTPServer, error) { // Create HTTP server with timeouts httpServer := http.Server{ Addr: srv.Addr, - Handler: gzip(mux), + Handler: handlers.CompressHandler(mux), ConnState: makeConnState(config.TLSConfig.EnableHTTP, handshakeTimeout, maxConns), ErrorLog: newHTTPServerLogger(srv.logger), } diff --git a/go.mod b/go.mod index da71b424c020..980101f08ea9 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,6 @@ go 1.17 // Pinned dependencies are noted in github.com/hashicorp/nomad/issues/11826 replace ( github.com/Microsoft/go-winio => github.com/endocrimes/go-winio v0.4.13-0.20190628114223-fb47a8b41948 - github.com/NYTimes/gziphandler => github.com/NYTimes/gziphandler v1.0.0 github.com/apparentlymart/go-textseg/v12 => github.com/apparentlymart/go-textseg/v12 v12.0.0 github.com/hashicorp/go-discover => github.com/hashicorp/go-discover v0.0.0-20210818145131-c573d69da192 github.com/hashicorp/hcl => github.com/hashicorp/hcl v1.0.1-0.20201016140508-a07e7d50bbee @@ -18,7 +17,6 @@ replace github.com/hashicorp/nomad/api => ./api require ( github.com/LK4D4/joincontext v0.0.0-20171026170139-1724345da6d5 github.com/Microsoft/go-winio v0.4.17 - github.com/NYTimes/gziphandler v1.0.1 github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e github.com/armon/go-metrics v0.3.10 github.com/aws/aws-sdk-go v1.42.27 @@ -41,6 +39,7 @@ require ( github.com/golang/protobuf v1.5.2 github.com/golang/snappy v0.0.4 github.com/google/go-cmp v0.5.6 + github.com/gorilla/handlers v1.5.1 github.com/gorilla/websocket v1.4.2 github.com/gosuri/uilive v0.0.4 github.com/grpc-ecosystem/go-grpc-middleware v1.2.1-0.20200228141219-3ce3d519df39 @@ -179,6 +178,7 @@ require ( github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 // indirect github.com/envoyproxy/go-control-plane v0.10.0 // indirect github.com/envoyproxy/protoc-gen-validate v0.6.2 // indirect + github.com/felixge/httpsnoop v1.0.1 // indirect github.com/go-ole/go-ole v1.2.4 // indirect github.com/godbus/dbus/v5 v5.0.4 // indirect github.com/gogo/protobuf v1.3.2 // indirect diff --git a/go.sum b/go.sum index db3894de9631..e0738ae83f20 100644 --- a/go.sum +++ b/go.sum @@ -114,8 +114,8 @@ github.com/Microsoft/hcsshim v0.8.23 h1:47MSwtKGXet80aIn+7h4YI6fwPmwIghAnsx2aOUr github.com/Microsoft/hcsshim v0.8.23/go.mod h1:4zegtUJth7lAvFyc6cH2gGQ5B3OFQim01nnU2M8jKDg= github.com/Microsoft/hcsshim/test v0.0.0-20201218223536-d3e5debf77da/go.mod h1:5hlzMzRKMLyo42nCZ9oml8AdTlq/0cvIaBv6tK1RehU= github.com/Microsoft/hcsshim/test v0.0.0-20210227013316-43a75bb4edd3/go.mod h1:mw7qgWloBUl75W/gVH3cQszUg1+gUITj7D6NY7ywVnY= -github.com/NYTimes/gziphandler v1.0.0 h1:OswZCvpiFsNRCbeapdJxDuikAqVXTgV7XAht8S9olZo= -github.com/NYTimes/gziphandler v1.0.0/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ= +github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ= +github.com/NYTimes/gziphandler v1.0.1/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= github.com/PuerkitoBio/purell v1.0.0/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0= @@ -445,6 +445,8 @@ github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w= github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk= github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M= +github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8SPQ= +github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k= github.com/frankban/quicktest v1.4.0/go.mod h1:36zfPVQyHxymz4cH7wlDmVwDrJuljRB60qkgn7rorfQ= github.com/frankban/quicktest v1.10.0/go.mod h1:ui7WezCLWMWxVWr1GETZY3smRy0G4KWq9vcPtJmFl7Y= @@ -618,6 +620,8 @@ github.com/gophercloud/gophercloud v0.1.0 h1:P/nh25+rzXouhytV2pUHBb65fnds26Ghl8/ github.com/gophercloud/gophercloud v0.1.0/go.mod h1:vxM41WHh5uqHVBMZHzuwNOHh8XEoIEcSTewFxm1c5g8= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= github.com/gorilla/handlers v0.0.0-20150720190736-60c7bfde3e33/go.mod h1:Qkdc/uu4tH4g6mTK6auzZ766c4CA0Ng8+o/OAirnOIQ= +github.com/gorilla/handlers v1.5.1 h1:9lRY6j8DEeeBT10CvO9hGW0gmky0BprnvDI5vfhUHH4= +github.com/gorilla/handlers v1.5.1/go.mod h1:t8XrUpc4KVXb7HGyJ4/cEnwQiaxrX/hz1Zv/4g96P1Q= github.com/gorilla/mux v1.7.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs= github.com/gorilla/mux v1.7.4/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI=