Merge pull request #11843 from hashicorp/deps-swap-gzip-handler

deps: replace gzip handler

.changelog/11843.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+deps: use gorilla package for gzip http handler
+```

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

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)
+}

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),
 		}

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

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=