fix nil panic in confighttp (open-telemetry#5628)

* fix nil panic in confighttp Signed-off-by: Ziqi Zhao <[email protected]> * add unit test and changelog Signed-off-by: Ziqi Zhao <[email protected]> * fix the review comments to use httptest package Signed-off-by: Ziqi Zhao <[email protected]> * move changelog to right place Signed-off-by: Ziqi Zhao <[email protected]> * add tests to cover different errors Signed-off-by: Ziqi Zhao <[email protected]> * fix unit test Signed-off-by: Ziqi Zhao <[email protected]> * fix unit test Signed-off-by: Ziqi Zhao <[email protected]>
mantian2020 · Jul 19, 2022 · 47b1ed6 · 47b1ed6
1 parent 9cb33dd
commit 47b1ed6
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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