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

Analyzer for http.Request #26

Open
bombsimon opened this issue Feb 18, 2020 · 0 comments
Open

Analyzer for http.Request #26

bombsimon opened this issue Feb 18, 2020 · 0 comments

Comments

@bombsimon
Copy link

bombsimon commented Feb 18, 2020

The current version of bodyclose only run analysis on http.Response. It's quite common that people use the same behaviour on http.Request (on server side) which is not really necessary. According to the documentation for http.Request, the Body field has the following comment:

// For server requests, the Request Body is always non-nil
// but will return EOF immediately when no body is present.
// The Server will close the request body. The ServeHTTP
// Handler does not need to.
Body io.ReadCloser

(Also confirmed by this SO post)

The reason to close the http.Response body can be found in the documentation for 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.

And documentation for http.Response Body field:

// The http Client and Transport guarantee that Body is always
// non-nil, even on responses without a body or responses with
// a zero-length body. It is the caller's responsibility to
// close Body. The default HTTP client's Transport may not
// reuse HTTP/1.x "keep-alive" TCP connections if the Body is
// not read to completion and closed.

Since that does not apply to the server side, this means that code like this is not needed:

func handle(w http.ResponseWriter, r *http.Request) {
    body, err := ioutl.ReadAll(r.Body)
    if err != nil {
        panic(err)
    }
    r.Body.Close() // <- Not needed
}

It would be nice if the linter was telling you to remove the line closing the http.Request body. If you agree, are you open to pull requests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant