Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

data-source/http: Ensure HTTP request body is not sent unless configured #390

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Feb 29, 2024

Closes #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: https://github.com/hashicorp/terraform-provider-http/issues/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.

Reference: #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: #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.
@bflad bflad added the bug label Feb 29, 2024
@bflad bflad added this to the v3.4.2 milestone Feb 29, 2024
@bflad bflad requested a review from a team as a code owner February 29, 2024 20:21
Copy link
Member

@austinvalle austinvalle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@bflad bflad merged commit b7aafe8 into main Feb 29, 2024
42 checks passed
@bflad bflad deleted the bflad/empty-request-body branch February 29, 2024 21:10
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error making requests to Cloudflare
2 participants