deps: adjust to gzip handler zero length response body

After swapping gzip handler to use the gorilla library, we
must account for a quirk in how zero/minimal length response
bodies are delivered.

The previous gzip handler was configured to compress all responses
regardless of size - even if the data was zero length or below the
network MTU. This behavior changed in [v1.1.0](c551b6c3b4 (diff-de723e6602cc2f16f7a9d85fd89d69954edc12a49134dab8901b10ee06d1879d))
which is why we could not upgrade.

The Nomad HTTP Client mutates the http.Response.Body object, making
a strong assumption that if the Content-Encoding header is set to "gzip",
the response will be readable via gzip decoder. This is no longer true
for the nytimes gzip handler, and is also not true for the gorilla gzip
handler.

It seems in practice this only makes a difference on the /v1/operator/license
endpoint which returns an empty response in OSS Nomad.

The fix here is to simply not wrap the response body reader if we
encounter an io.EOF while creating the gzip reader - indicating there
is no data to decode.
This commit is contained in:
Seth Hoenig 2022-01-13 09:57:36 -06:00
parent 2a5f7c0386
commit 9a6988f55b
3 changed files with 83 additions and 19 deletions

3
.changelog/11843.txt Normal file
View File

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

View File

@ -741,35 +741,47 @@ func (c *Client) doRequest(r *request) (time.Duration, *http.Response, error) {
if err != nil { if err != nil {
return 0, nil, err return 0, nil, err
} }
start := time.Now() start := time.Now()
resp, err := c.httpClient.Do(req) resp, err := c.httpClient.Do(req)
diff := time.Since(start) diff := time.Since(start)
// If the response is compressed, we swap the body's reader. // If the response is compressed, we swap the body's reader.
if resp != nil && resp.Header != nil { if zipErr := c.autoUnzip(resp); zipErr != nil {
var reader io.ReadCloser return 0, nil, zipErr
switch resp.Header.Get("Content-Encoding") {
case "gzip":
greader, err := gzip.NewReader(resp.Body)
if err != nil {
return 0, nil, err
}
// 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
}
resp.Body = reader
} }
return diff, resp, err 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},
}
}
return nil
}
// rawQuery makes a GET request to the specified endpoint but returns just the // rawQuery makes a GET request to the specified endpoint but returns just the
// response body. // response body.
func (c *Client) rawQuery(endpoint string, q *QueryOptions) (io.ReadCloser, error) { func (c *Client) rawQuery(endpoint string, q *QueryOptions) (io.ReadCloser, error) {

View File

@ -1,10 +1,13 @@
package api package api
import ( import (
"bytes"
"compress/gzip"
"context" "context"
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
"io"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"net/url" "net/url"
@ -568,3 +571,49 @@ func TestClient_HeaderRaceCondition(t *testing.T) {
require.Equal(2, <-c, "goroutine request should have two headers") require.Equal(2, <-c, "goroutine request should have two headers")
require.Len(conf.Headers, 1, "config headers should not mutate") 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)
}