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

[BUG] - HTTP request is not being closed properly #254

Open
optik-aper opened this issue Jul 14, 2023 · 0 comments
Open

[BUG] - HTTP request is not being closed properly #254

optik-aper opened this issue Jul 14, 2023 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@optik-aper
Copy link
Member

Describe the bug
The NopCloser doesn't actually close the request Body which annoys static analyzers and is probably going to create issues. Research whether the http library needs the response closed and what we can do to copy the response in a way that is useful to the new functionality exposing the request.

To Reproduce
Update DoWithContext's defer func in govultr.go to do something like this:

	defer func() {
		if err := res.Body.Close(); err != nil {
			fmt.Printf("Error: %v\n", err)
		}

		if bod, err := res.Body.Read(nil); err == nil {
			fmt.Printf("Body: %v\n", bod)
		}
	}()

It shouldn't be able to read the Body, but, the request is replaced by a NopCloser via:

	res.Body = io.NopCloser(bytes.NewBuffer(body))

The NopCloser actually just returns nil when you try to close it. https://cs.opensource.google/go/go/+/refs/tags/go1.20.6:src/io/io.go;l=678

This was added to allow us to return the request for use in the package, but, I'm concerned that it's going to leave connections open. Will need to test this further at some point.

Expected behavior
All http requests will close after completion

Desktop (please complete the following information where applicable:

  • OS: ubuntu 22.04
  • Language Version: go 1.20
  • Browser: na
  • Version: v3.1.0

Additional context

Golanci-lint calls bodyclose which tipped me off that something wasn't quite right here.

block_storage.go:121:33: response body must be closed (bodyclose)
	_, err = b.client.DoWithContext(ctx, req, nil)
	                               ^
block_storage.go:133:33: response body must be closed (bodyclose)
	_, err = b.client.DoWithContext(ctx, req, nil)
	                               ^
block_storage.go:171:33: response body must be closed (bodyclose)
	_, err = b.client.DoWithContext(ctx, req, nil)                  
@optik-aper optik-aper added the bug Something isn't working label Jul 14, 2023
@optik-aper optik-aper self-assigned this Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant