From a55758020f2514297f722dbc2f51c036e95bf794 Mon Sep 17 00:00:00 2001
From: Abin Simon <abinsimon10@gmail.com>
Date: Tue, 16 May 2023 10:16:56 +0530
Subject: [PATCH] Ensure response body gets closed (#3409)

From https://pkg.go.dev/net/http#Client.Do

> If the returned error is nil, the Response will contain a non-nil
> Body which the user is expected to close. If the Body is not both read
> to EOF and closed, the Client's underlying RoundTripper (typically
> Transport) may not be able to re-use a persistent TCP connection to
> the server for a subsequent "keep-alive" request.

These were found using https://github.com/timakin/bodyclose. Not adding it as a linter for now as it was producing false positive(I think) on OneDrive item. The close was happening in https://github.com/alcionai/corso/blob/a9918d2f78b9419af553a6e0356bbf5f6078b9d8/src/internal/kopia/upload.go#L68 instead of right alongside it which might be why it got thrown off.

<!-- PR description-->

---

#### Does this PR need a docs update or release note?

- [ ] :white_check_mark: Yes, it's included
- [ ] :clock1: Yes, but in a later PR
- [x] :no_entry: No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] :sunflower: Feature
- [x] :bug: Bugfix
- [ ] :world_map: Documentation
- [ ] :robot: Supportability/Tests
- [ ] :computer: CI/Deployment
- [ ] :broom: Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* #<issue>

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [x] :muscle: Manual
- [ ] :zap: Unit test
- [ ] :green_heart: E2E
---
 src/cmd/getM365/onedrive/get_item.go                      | 1 +
 .../connector/graph/concurrency_middleware_test.go        | 3 +++
 src/internal/connector/graph/http_wrapper_test.go         | 8 ++++++--
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/cmd/getM365/onedrive/get_item.go b/src/cmd/getM365/onedrive/get_item.go
index 3b338ca74..9e16ee746 100644
--- a/src/cmd/getM365/onedrive/get_item.go
+++ b/src/cmd/getM365/onedrive/get_item.go
@@ -198,6 +198,7 @@ func getDriveItemContent(
 	if err != nil {
 		return nil, clues.New("downloading item").With("error", err)
 	}
+	defer resp.Body.Close()
 
 	content, err := io.ReadAll(resp.Body)
 	if err != nil {
diff --git a/src/internal/connector/graph/concurrency_middleware_test.go b/src/internal/connector/graph/concurrency_middleware_test.go
index 4e7e57606..c5734a665 100644
--- a/src/internal/connector/graph/concurrency_middleware_test.go
+++ b/src/internal/connector/graph/concurrency_middleware_test.go
@@ -62,6 +62,9 @@ func (suite *ConcurrencyLimiterUnitTestSuite) TestConcurrencyLimiter() {
 
 			resp, err := client.Get(ts.URL)
 			require.NoError(t, err)
+
+			defer resp.Body.Close()
+
 			assert.Equal(t, http.StatusOK, resp.StatusCode)
 		}()
 	}
diff --git a/src/internal/connector/graph/http_wrapper_test.go b/src/internal/connector/graph/http_wrapper_test.go
index 40abea977..92348736c 100644
--- a/src/internal/connector/graph/http_wrapper_test.go
+++ b/src/internal/connector/graph/http_wrapper_test.go
@@ -40,8 +40,10 @@ func (suite *HTTPWrapperIntgSuite) TestNewHTTPWrapper() {
 		"https://www.corsobackup.io",
 		nil,
 		nil)
-
 	require.NoError(t, err, clues.ToCore(err))
+
+	defer resp.Body.Close()
+
 	require.NotNil(t, resp)
 	require.Equal(t, http.StatusOK, resp.StatusCode)
 }
@@ -108,8 +110,10 @@ func (suite *HTTPWrapperUnitSuite) TestNewHTTPWrapper_redirectMiddleware() {
 	hw := NewHTTPWrapper(appendMiddleware(&mwResp))
 
 	resp, err := hw.Request(ctx, http.MethodGet, url, nil, nil)
-
 	require.NoError(t, err, clues.ToCore(err))
+
+	defer resp.Body.Close()
+
 	require.NotNil(t, resp)
 	// require.Equal(t, 1, calledCorrectly, "test server was called with expected path")
 	require.Equal(t, http.StatusOK, resp.StatusCode)