From 39d4638ae636a6431c49182b1a7249848d6181c6 Mon Sep 17 00:00:00 2001
From: Seth Hoenig <seth.hoenig@hashicorp.com>
Date: Thu, 13 Jan 2022 09:57:36 -0600
Subject: [PATCH] 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](https://github.com/nytimes/gziphandler/commit/c551b6c3b4b976eafa1a18220b5e21692784d8e2#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.
---
 api/api.go      | 48 ++++++++++++++++++++++++++++++------------------
 api/api_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 18 deletions(-)

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