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

Jump to bottom

net/http/httputil: New method ReadBody(*http.Response) ([]byte, error) #15915

Closed
erikdubbelboer opened this issue Jun 1, 2016 · 3 comments
Closed

net/http/httputil: New method ReadBody(*http.Response) ([]byte, error) #15915

erikdubbelboer opened this issue Jun 1, 2016 · 3 comments
Labels
FrozenDueToAge Proposal
Milestone
Go1.8Maybe

Comments

@erikdubbelboer
Copy link
Contributor

I propose a new, simple, but very useful method for net/http/httputil:

func ReadBody(res *http.Response) ([]byte, error) {
  // A ContentLength value of -1 indicates that the length is unknown.
  if res.ContentLength == -1 {
    // Fall back to ReadAll.
    return ioutil.ReadAll(res.Body)
  } else {
    body := make([]byte, res.ContentLength)

    // No need to check the number of bytes read as err will
    // be != nil if it's less than ContentLength.
    _, err := io.ReadFull(res.Body, body)
    return body, err
  }
}

Usually people use ioutil.ReadAll to read the body of an http response. This will result in at least one garbaged bytes.Buffer and some over allocating of the resulting []byte. This new method is similar to ioutil.ReadAll only it uses the ContentLength to only allocate the number of bytes really needed.
@ianlancetaylor ianlancetaylor added this to the Go1.8Maybe milestone Jun 1, 2016
@erikdubbelboer
Copy link
Contributor Author

I guess it should be two separate methods such as ReadRequestBody and ReadResponseBody. Or just one method with an interface{} as argument but I don't think that's something done much in the standard library.

@odeke-em
Copy link
Member

odeke-em commented Oct 3, 2016

@bradfitz what is your view on this proposal? Accepted or vetoed?

@bradfitz
Copy link
Contributor

bradfitz commented Oct 3, 2016

I don't like the idea of encouraging more ReadAll-sorts of interfaces, especially here where the ContentLength isn't under your control or even known.

Also, this doesn't respect StatusCode as an error.

I do like the idea of reducing some of the boilerplate associated with HTTP, but I don't think this is the answer. I want to see something which lets people declare limits (perhaps with a sane default if undeclared) and also lets them respect HTTP status codes as well.

Also, it should be a method on Response or Client, not a top-level function.

@rsc rsc closed this as completed Oct 20, 2016
@golang golang locked and limited conversation to collaborators Oct 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
None yet
Milestone
Go1.8Maybe
Development

No branches or pull requests
6 participants
@bradfitz @rsc @erikdubbelboer @ianlancetaylor @odeke-em @gopherbot