Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps: replace gzip handler #11843

Merged
merged 2 commits into from
Jan 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/11843.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
deps: use gorilla package for gzip http handler
```
48 changes: 30 additions & 18 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 49 additions & 0 deletions api/api_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package api

import (
"bytes"
"compress/gzip"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"net/http/httptest"
"net/url"
Expand Down Expand Up @@ -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)
}
10 changes: 2 additions & 8 deletions command/agent/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
}
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these be removed?

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=
Expand Down Expand Up @@ -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=
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I correct that this is indirectly required by Gorilla or did we add these?

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=
Expand Down Expand Up @@ -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=
Expand Down