response body must be closed (bodyclose) when net/http is imported

[Crocmagnon]

Hello! 👋🏻

Not sure "bug" is the most appropriate category but it didn't fit in any other one (I didn't find "question") so here it is.

The following piece of code triggers bodyclose:

package main

import (
	"bytes"
	"context"
	"net/http"

	"github.com/carlmjohnson/requests"
)

func main() {
	var errResp bytes.Buffer

	_ = requests.URL("https://example.com/").
		Method(http.MethodPost).
		AddValidator(requests.ValidatorHandler(requests.DefaultValidator, requests.ToBytesBuffer(&errResp))).
		Fetch(context.Background())
}

$ golangci-lint run --enable bodyclose ./...
main.go:16:91: response body must be closed (bodyclose)
                AddValidator(requests.ValidatorHandler(requests.DefaultValidator, requests.ToBytesBuffer(&errResp))).

Interestingly, when removing the net/http import (for example by removing the Method or replacing it with an equivalent string), the issue disappears.

My questions would be:

• Is the response body properly closed? In which case the linter warning is a false positive.
• Is it not? If so, the linter should probably trigger even if net/http isn't imported, and requests should probably be fixed to handle the situation.

[earthboundkid]

This is almost certainly a false positive. All request-responses go through the do function, which ensures Body.Close is called.

func do(cl *http.Client, req *http.Request, validators []ResponseHandler, h ResponseHandler) (doResponse, error) {
        res, err := cl.Do(req)
        if err != nil {
                return doConnect, err
        }
        defer res.Body.Close()


        for _, v := range validators {
                if v == nil {
                        continue
                }
                if err = v(res); err != nil {
                        return doValidate, err
                }
        }
        if err = h(res); err != nil {
                return doHandle, err
        }


        return doOK, nil
}

https://github.com/earthboundkid/requests/blob/main/core_do.go#L16-L36

I think the bug should be filed with the linter. I don't know what they are looking for, but it seems to be keying off the wrong thing in the requests package.

[Crocmagnon]

Thanks, I'll do that 👍🏻

[Crocmagnon]

timakin/bodyclose#71

[aphsa]

I do get many bodyclose findings in the package itself. All of these are false positives?

builder_core.go:187:24: response body must be closed (bodyclose)
builder_core.go:256:20: response body must be closed (bodyclose)
builder_extras.go:168:36: response body must be closed (bodyclose)
builder_extras.go:173:41: response body must be closed (bodyclose)
builder_extras.go:178:34: response body must be closed (bodyclose)
builder_extras.go:185:24: response body must be closed (bodyclose)
builder_extras.go:192:25: response body must be closed (bodyclose)
builder_extras.go:197:27: response body must be closed (bodyclose)
builder_extras.go:202:32: response body must be closed (bodyclose)
builder_extras.go:207:27: response body must be closed (bodyclose)
builder_extras.go:214:25: response body must be closed (bodyclose)
builder_extras.go:223:27: response body must be closed (bodyclose)
builder_extras.go:231:23: response body must be closed (bodyclose)
builder_extras.go:231:35: response body must be closed (bodyclose)
builder_extras.go:238:34: response body must be closed (bodyclose)
handler.go:59:23: response body must be closed (bodyclose)
handler.go:100:22: response body must be closed (bodyclose)
handler.go:112:22: response body must be closed (bodyclose)
handler_example_test.go:21:32: response body must be closed (bodyclose)
handler_example_test.go:51:33: response body must be closed (bodyclose)
handler_example_test.go:74:25: response body must be closed (bodyclose)
reqhtml/html_test.go:26:20: response body must be closed (bodyclose)
reqxml/err_example_test.go:25:28: response body must be closed (bodyclose)
reqxml/to_example_test.go:32:19: response body must be closed (bodyclose)
validator_handler.go:33:25: response body must be closed (bodyclose)
validator_handler.go:33:50: response body must be closed (bodyclose)
validator_handler_example_test.go:25:29: response body must be closed (bodyclose)
validator_handler_example_test.go:27:22: response body must be closed (bodyclose)

[earthboundkid]

Yes, they are all false positives.