diff --git a/.changes/unreleased/BUG FIXES-20240229-151134.yaml b/.changes/unreleased/BUG FIXES-20240229-151134.yaml new file mode 100644 index 00000000..4ced9174 --- /dev/null +++ b/.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" diff --git a/.changes/unreleased/NOTES-20240229-151426.yaml b/.changes/unreleased/NOTES-20240229-151426.yaml new file mode 100644 index 00000000..196c44d0 --- /dev/null +++ b/.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" diff --git a/internal/provider/data_source_http.go b/internal/provider/data_source_http.go index 7bfa1e11..f6cb5508 100644 --- a/internal/provider/data_source_http.go +++ b/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) diff --git a/internal/provider/data_source_http_test.go b/internal/provider/data_source_http_test.go index befd9912..d10dde53 100644 --- a/internal/provider/data_source_http_test.go +++ b/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