Fix error reading closed response body on redirect (#6372)

plugins/inputs/http_response/http_response.go

@@ -141,7 +141,7 @@ func (h *HTTPResponse) createHttpClient() (*http.Client, error) {
 
 	if h.FollowRedirects == false {
 		client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
-			return ErrRedirectAttempted
+			return http.ErrUseLastResponse
 		}
 	}
 	return client, nil
@@ -254,17 +254,8 @@ func (h *HTTPResponse) httpGather(u string) (map[string]interface{}, map[string]
 
 		// Any error not recognized by `set_error` is considered a "connection_failed"
 		setResult("connection_failed", fields, tags)
-
-		// If the error is a redirect we continue processing and log the HTTP code
-		urlError, isUrlError := err.(*url.Error)
-		if !h.FollowRedirects && isUrlError && urlError.Err == ErrRedirectAttempted {
-			err = nil
-		} else {
-			// If the error isn't a timeout or a redirect stop
-			// processing the request
 		return fields, tags, nil
 	}
-	}
 
 	if _, ok := fields["response_time"]; !ok {
 		fields["response_time"] = response_time

plugins/inputs/http_response/http_response_test.go

@@ -10,9 +10,9 @@ import (
 	"testing"
 	"time"
 
+	"github.com/influxdata/telegraf"
 	"github.com/influxdata/telegraf/internal"
 	"github.com/influxdata/telegraf/testutil"
-
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -36,6 +36,7 @@ func checkAbsentTags(t *testing.T, tags []string, acc *testutil.Accumulator) {
 // Receives a dictionary and with expected fields and their values. If a value is nil, it will only check
 // that the field exists, but not its contents
 func checkFields(t *testing.T, fields map[string]interface{}, acc *testutil.Accumulator) {
+	t.Helper()
 	for key, field := range fields {
 		switch v := field.(type) {
 		case int:
@@ -121,6 +122,7 @@ func setUpTestMux() http.Handler {
 }
 
 func checkOutput(t *testing.T, acc *testutil.Accumulator, presentFields map[string]interface{}, presentTags map[string]interface{}, absentFields []string, absentTags []string) {
+	t.Helper()
 	if presentFields != nil {
 		checkFields(t, presentFields, acc)
 	}
@@ -651,12 +653,11 @@ func TestTimeout(t *testing.T) {
 	checkOutput(t, &acc, expectedFields, expectedTags, absentFields, absentTags)
 }
 
-func TestPluginErrors(t *testing.T) {
+func TestBadRegex(t *testing.T) {
 	mux := setUpTestMux()
 	ts := httptest.NewServer(mux)
 	defer ts.Close()
 
-	// Bad regex test. Should return an error and return nothing
 	h := &HTTPResponse{
 		Address:             ts.URL + "/good",
 		Body:                "{ 'test': 'data'}",
@@ -676,36 +677,6 @@ func TestPluginErrors(t *testing.T) {
 	absentFields := []string{"http_response_code", "response_time", "content_length", "response_string_match", "result_type", "result_code"}
 	absentTags := []string{"status_code", "result", "server", "method"}
 	checkOutput(t, &acc, nil, nil, absentFields, absentTags)
-
-	// Attempt to read empty body test
-	h = &HTTPResponse{
-		Address:             ts.URL + "/redirect",
-		Body:                "",
-		Method:              "GET",
-		ResponseStringMatch: ".*",
-		ResponseTimeout:     internal.Duration{Duration: time.Second * 20},
-		FollowRedirects:     false,
-	}
-
-	acc = testutil.Accumulator{}
-	err = h.Gather(&acc)
-	require.NoError(t, err)
-
-	expectedFields := map[string]interface{}{
-		"http_response_code":    http.StatusMovedPermanently,
-		"response_string_match": 0,
-		"result_type":           "body_read_error",
-		"result_code":           2,
-		"response_time":         nil,
-		"content_length":        nil,
-	}
-	expectedTags := map[string]interface{}{
-		"server":      nil,
-		"method":      "GET",
-		"status_code": "301",
-		"result":      "body_read_error",
-	}
-	checkOutput(t, &acc, expectedFields, expectedTags, nil, nil)
 }
 
 func TestNetworkErrors(t *testing.T) {
@@ -827,3 +798,50 @@ func TestContentLength(t *testing.T) {
 	absentFields = []string{"response_string_match"}
 	checkOutput(t, &acc, expectedFields, expectedTags, absentFields, nil)
 }
+
+func TestRedirect(t *testing.T) {
+	ts := httptest.NewServer(http.NotFoundHandler())
+	defer ts.Close()
+
+	ts.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Add("Location", "http://example.org")
+		w.WriteHeader(http.StatusMovedPermanently)
+		w.Write([]byte("test"))
+	})
+
+	plugin := &HTTPResponse{
+		URLs:                []string{ts.URL},
+		ResponseStringMatch: "test",
+	}
+
+	var acc testutil.Accumulator
+	err := plugin.Gather(&acc)
+	require.NoError(t, err)
+
+	expected := []telegraf.Metric{
+		testutil.MustMetric(
+			"http_response",
+			map[string]string{
+				"server":      ts.URL,
+				"method":      "GET",
+				"result":      "success",
+				"status_code": "301",
+			},
+			map[string]interface{}{
+				"result_code":           0,
+				"result_type":           "success",
+				"http_response_code":    301,
+				"response_string_match": 1,
+				"content_length":        4,
+			},
+			time.Unix(0, 0),
+		),
+	}
+
+	actual := acc.GetTelegrafMetrics()
+	for _, m := range actual {
+		m.RemoveField("response_time")
+	}
+
+	testutil.RequireMetricsEqual(t, expected, actual, testutil.IgnoreTime())
+}