data-source/http: Ensure HTTP request body is not sent unless configu…

…red (hashicorp#390)

Reference: hashicorp#388

Previously the HTTP request would unexpectedly always contain a body for all requests. Certain HTTP server implementations are sensitive to this data existing if it is not expected. Requests now only contain a request body if the `request_body` attribute is explicitly set. To exactly preserve the previous behavior, set `request_body = ""`.

Added new acceptance testing for `request_body` handling. Go net/http server implementations seem to ignore this problematic situation (or it was not easy to determine), so also verified with a real world configuration using a temporary acceptance test as well as manually running Terraform using a development build containing these changes:

```
// Reference: hashicorp#388
func TestDataSource_RequestBody_Null(t *testing.T) {
	t.Parallel()

	resource.Test(t, resource.TestCase{
		ProtoV5ProviderFactories: protoV5ProviderFactories(),
		Steps: []resource.TestStep{
			{
				Config: `
					data "http" "test" {
						url = "https://www.cloudflare.com/ips-v4"
					}`,
				Check: resource.ComposeAggregateTestCheckFunc(
					resource.TestCheckResourceAttr("data.http.test", "status_code", "200"),
				),
			},
		},
	})
}
```

This real world acceptance test is not added as the server may change its implementation in the future.

.changes/unreleased/BUG FIXES-20240229-151134.yaml

@@ -0,0 +1,5 @@
+kind: BUG FIXES
+body: 'data-source/http: Ensured HTTP request body is not sent unless configured'
+time: 2024-02-29T15:11:34.198556-05:00
+custom:
+  Issue: "388"

.changes/unreleased/NOTES-20240229-151426.yaml

@@ -0,0 +1,9 @@
+kind: NOTES
+body: 'data-source/http: Previously the HTTP request would unexpectedly always contain
+  a body for all requests. Certain HTTP server implementations are sensitive to this
+  data existing if it is not expected. Requests now only contain a request body if
+  the `request_body` attribute is explicitly set. To exactly preserve the previous
+  behavior, set `request_body = ""`.'
+time: 2024-02-29T15:14:26.883908-05:00
+custom:
+  Issue: "388"

internal/provider/data_source_http.go

@@ -200,7 +200,6 @@ func (d *httpDataSource) Read(ctx context.Context, req datasource.ReadRequest, r
 	requestURL := model.URL.ValueString()
 	method := model.Method.ValueString()
 	requestHeaders := model.RequestHeaders
-	requestBody := strings.NewReader(model.RequestBody.ValueString())
 
 	if method == "" {
 		method = "GET"
@@ -284,7 +283,8 @@ func (d *httpDataSource) Read(ctx context.Context, req datasource.ReadRequest, r
 		retryClient.RetryWaitMax = time.Duration(retry.MaxDelay.ValueInt64()) * time.Millisecond
 	}
 
-	request, err := retryablehttp.NewRequestWithContext(ctx, method, requestURL, requestBody)
+	request, err := retryablehttp.NewRequestWithContext(ctx, method, requestURL, nil)
+
 	if err != nil {
 		resp.Diagnostics.AddError(
 			"Error creating request",
@@ -293,6 +293,19 @@ func (d *httpDataSource) Read(ctx context.Context, req datasource.ReadRequest, r
 		return
 	}
 
+	if !model.RequestBody.IsNull() {
+		err = request.SetBody(strings.NewReader(model.RequestBody.ValueString()))
+
+		if err != nil {
+			resp.Diagnostics.AddError(
+				"Error Setting Request Body",
+				"An unexpected error occurred while setting the request body: "+err.Error(),
+			)
+
+			return
+		}
+	}
+
 	for name, value := range requestHeaders.Elements() {
 		var header string
 		diags = tfsdk.ValueAs(ctx, value, &header)

internal/provider/data_source_http_test.go

@@ -7,6 +7,7 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"net/http/httputil"
@@ -798,6 +799,71 @@ func TestDataSource_MaxDelayAtLeastEqualToMinDelay(t *testing.T) {
 	})
 }
 
+// Reference: https://github.com/hashicorp/terraform-provider-http/issues/388
+func TestDataSource_RequestBody(t *testing.T) {
+	t.Parallel()
+
+	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/plain")
+
+		requestBody, err := io.ReadAll(r.Body)
+
+		if err != nil {
+			w.WriteHeader(http.StatusInternalServerError)
+			_, _ = w.Write([]byte(`Request Body Read Error: ` + err.Error()))
+
+			return
+		}
+
+		// If the request body is empty or a test string, return a 200 OK with a response body.
+		if len(requestBody) == 0 || string(requestBody) == "test" {
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`test response body`))
+
+			return
+		}
+
+		w.WriteHeader(http.StatusBadRequest)
+		_, _ = w.Write([]byte(`request body (` + string(requestBody) + `) was not empty or "test"`))
+	}))
+	defer svr.Close()
+
+	resource.UnitTest(t, resource.TestCase{
+		ProtoV5ProviderFactories: protoV5ProviderFactories(),
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(`
+					data "http" "test" {
+						url = "%s"
+					}`, svr.URL),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("data.http.test", "status_code", "200"),
+				),
+			},
+			{
+				Config: fmt.Sprintf(`
+					data "http" "test" {
+						request_body = "test"
+						url          = %q
+					}`, svr.URL),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("data.http.test", "status_code", "200"),
+				),
+			},
+			{
+				Config: fmt.Sprintf(`
+					data "http" "test" {
+						request_body = "not-test"
+						url          = %q
+					}`, svr.URL),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("data.http.test", "status_code", "400"),
+				),
+			},
+		},
+	})
+}
+
 func TestDataSource_ResponseBodyText(t *testing.T) {
 	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		_, _ = w.Write([]byte(`你好世界`)) // Hello world