From 47b1ed6e590cbbb5c716e7b6fc25bb41c6a8b39d Mon Sep 17 00:00:00 2001
From: Ziqi Zhao <zhaoziqi9146@gmail.com>
Date: Wed, 20 Jul 2022 02:54:10 +0800
Subject: [PATCH] fix nil panic in confighttp (#5628)

* fix nil panic in confighttp

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

* add unit test and changelog

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

* fix the review comments to use httptest package

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

* move changelog to right place

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

* add tests to cover different errors

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

* fix unit test

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

* fix unit test

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>
---
 CHANGELOG.md                          |  1 +
 config/confighttp/compression.go      | 20 +++---
 config/confighttp/compression_test.go | 88 +++++++++++++++++++++++++++
 3 files changed, 100 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d379f1b416a..0d54591194e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@
 
 - Fix Collector panic when disabling telemetry metrics (#5642)
 - Fix Collector panic when featuregate value is empty (#5663)
+- Fix confighttp.compression panic due to nil request.Body. (#5628)
 
 ## v0.55.0 Beta
 
diff --git a/config/confighttp/compression.go b/config/confighttp/compression.go
index 13537d599f1..e85b2cfa90c 100644
--- a/config/confighttp/compression.go
+++ b/config/confighttp/compression.go
@@ -82,19 +82,21 @@ func (r *compressRoundTripper) RoundTrip(req *http.Request) (*http.Response, err
 	if writerErr != nil {
 		return nil, writerErr
 	}
-	_, copyErr := io.Copy(compressWriter, req.Body)
-	closeErr := req.Body.Close()
+	if req.Body != nil {
+		_, copyErr := io.Copy(compressWriter, req.Body)
+		closeErr := req.Body.Close()
 
-	if err := compressWriter.Close(); err != nil {
-		return nil, err
-	}
+		if copyErr != nil {
+			return nil, copyErr
+		}
 
-	if copyErr != nil {
-		return nil, copyErr
+		if closeErr != nil {
+			return nil, closeErr
+		}
 	}
 
-	if closeErr != nil {
-		return nil, closeErr
+	if err := compressWriter.Close(); err != nil {
+		return nil, err
 	}
 
 	// Create a new request since the docs say that we cannot modify the "req"
diff --git a/config/confighttp/compression_test.go b/config/confighttp/compression_test.go
index 9a477559a2e..37aeaf5aff1 100644
--- a/config/confighttp/compression_test.go
+++ b/config/confighttp/compression_test.go
@@ -22,6 +22,8 @@ import (
 	"io/ioutil"
 	"net"
 	"net/http"
+	"net/http/httptest"
+	"net/url"
 	"testing"
 	"time"
 
@@ -233,6 +235,92 @@ func TestHTTPContentDecompressionHandler(t *testing.T) {
 	}
 }
 
+func TestHTTPContentCompressionRequestWithNilBody(t *testing.T) {
+	compressedGzipBody, _ := compressGzip([]byte{})
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(200)
+		body, err := ioutil.ReadAll(r.Body)
+		require.NoError(t, err, "failed to read request body: %v", err)
+		assert.EqualValues(t, compressedGzipBody.Bytes(), body)
+	}))
+	defer server.Close()
+
+	req, err := http.NewRequest("GET", server.URL, nil)
+	require.NoError(t, err, "failed to create request to test handler")
+
+	client := http.Client{}
+	client.Transport = newCompressRoundTripper(http.DefaultTransport, configcompression.Gzip)
+	res, err := client.Do(req)
+	require.NoError(t, err)
+
+	_, err = ioutil.ReadAll(res.Body)
+	require.NoError(t, err)
+	require.NoError(t, res.Body.Close(), "failed to close request body: %v", err)
+}
+
+type copyFailBody struct {
+}
+
+func (*copyFailBody) Read(p []byte) (n int, err error) {
+	return 0, fmt.Errorf("read failed")
+}
+
+func (*copyFailBody) Close() error {
+	return nil
+}
+
+func TestHTTPContentCompressionCopyError(t *testing.T) {
+	body := &copyFailBody{}
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(200)
+	}))
+	defer server.Close()
+
+	url, _ := url.Parse(server.URL)
+	req := &http.Request{
+		Method: "GET",
+		URL:    url,
+		Body:   body,
+	}
+
+	client := http.Client{}
+	client.Transport = newCompressRoundTripper(http.DefaultTransport, configcompression.Gzip)
+	_, err := client.Do(req)
+	require.Error(t, err)
+}
+
+type closeFailBody struct {
+	*bytes.Buffer
+}
+
+func (*closeFailBody) Close() error {
+	return fmt.Errorf("close failed")
+}
+
+func TestHTTPContentCompressionRequestBodyCloseError(t *testing.T) {
+	testBody := []byte("blank")
+	body := &closeFailBody{
+		Buffer: bytes.NewBuffer(testBody),
+	}
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(200)
+	}))
+	defer server.Close()
+
+	url, _ := url.Parse(server.URL)
+	req := &http.Request{
+		Method: "GET",
+		URL:    url,
+		Body:   body,
+	}
+
+	client := http.Client{}
+	client.Transport = newCompressRoundTripper(http.DefaultTransport, configcompression.Gzip)
+	_, err := client.Do(req)
+	require.Error(t, err)
+}
+
 func compressGzip(body []byte) (*bytes.Buffer, error) {
 	var buf bytes.Buffer