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

net/http: Allow custom io.Readers to provide a size for ContentLength in requests #6738

Open
cookieo9 opened this Issue Nov 8, 2013 · 8 comments

Comments

Projects
None yet
7 participants
@cookieo9
Contributor

cookieo9 commented Nov 8, 2013

Currently only 3 types can specify automatically the ContentLength of the body of an
http.Request:
 - bytes.Reader
 - bytes.Buffer
 - strings.Reader

In these cases, the *http.Request automatically has the ContentLength field set by the
value of the Len method of these three types. A user using the http package with any
other kind of io.Reader must manually call NewRequest, then set the ContentLength
themselves, and then call http.Client.Do.

If instead, if http.NewRequest was modified to support passing an io.Reader with a (for
example) Size() method, and then set the ContentLength to the value returned from that
function, users could avoid having to set the ContentLength field of the request
themselves. This would allow the http.Post() and http.Client.Post() helper methods to be
more used in more situations, if passed such an io.Reader. The bigger benefit would be
for external packages which use http though.

A situation has arisen with github package that makes http requests to the github API
where uploads must be sent a Content-Length, see:
https://groups.google.com/forum/#!topic/golang-nuts/Hipcz658QQ8

Currently the easiest way to upload files successfully is to load the entire file into a
[]byte in memory, create a bytes.Buffer or bytes.Reader from it, and pass that to the
package functions. With the proposed feature, an "augmented" os.File could be
used instead, saving a lot of memory and time for large files.

Although it would be possible to modify the go-github package to get a size (somehow)
for the data in the io.Reader and then set the Content-Length itself, by providing this
functionality in the standard library, then all packages which wrap the http package
could benefit.
@cookieo9

This comment has been minimized.

Contributor

cookieo9 commented Nov 8, 2013

Comment 1:

Alternatively, if http.NewRequest understood a few more std library io.Reader types,
such as:
 - io.LimitedReader
 - io.SectionReader
 - os.File
Then it wouldn't be necessary for users of the http package to create a new type to
satisfy the new interface. Also the scale of the change conceptually would be smaller.
@nightlyone

This comment has been minimized.

Contributor

nightlyone commented Nov 8, 2013

Comment 2:

Another very useful thing to solve this problem would by introducing a "io.SizedReader"
interface, which will be satisfied by every io.Reader which additionally knows it's own
size in bytes.
Whether one will use the Size() method or a new ByteSize() method is open to discussion.
This would also allow decompression readers to report the final size without trying to
be seekable.
@minux

This comment has been minimized.

Member

minux commented Nov 8, 2013

Comment 3:

i think supporting io.ReadSeeker suffices to cover io.SectionReader and os.File.
Then we only need to add support for io.LimitedReader.
For decompression readers, we can wrap them into a io.LimitedReader.
@bradfitz

This comment has been minimized.

Member

bradfitz commented Nov 13, 2013

Comment 4:

Not LimitedReader.  That's just a max.
*os.File maybe, but that requires statting and seeking to figure out where you're at.
Likewise with SectionReader, which doesn't require a stat but does require a Seek (but
the seek is guaranteed to succeed).  Also http://play.golang.org/p/Ee5I6oesXW makes me
nervous.
I'm nervous about seeking and statting, because for example: if those return an error,
does NewRequest return an error (unlike Go 1.[012]), or does it silently just not set a
Content-Length (surprising).  Neither answer is good.
Adding a new interface like interface { ContentLength() int64 } would infect or
encourage infection lots of types, so that's out.  Likewise with a hidden interface.
Adding io.SizedReader is close to interesting, but I actually want io.SizeReaderAt (see
http://talks.golang.org/2013/oscon-dl.slide#49) which I've used for a number of things
inside Google.  But even a *
I'm inclined to do nothing.

Status changed to Thinking.

@rsc

This comment has been minimized.

Contributor

rsc commented Nov 27, 2013

Comment 5:

Labels changed: added go1.3maybe.

@rsc

This comment has been minimized.

Contributor

rsc commented Dec 4, 2013

Comment 6:

Labels changed: added release-none, removed go1.3maybe.

@rsc

This comment has been minimized.

Contributor

rsc commented Dec 4, 2013

Comment 7:

Labels changed: added repo-main.

@heidawei

This comment has been minimized.

heidawei commented Feb 26, 2015

a io.Reader come from bytes.NewBuffer() or bytes.NewReader() is inefficiency when copy to a io.Writer,but the io.Reader come from http req.body(it is a io.Reader object) is efficiency.how to solve this problem??

@rsc rsc added this to the Unplanned milestone Apr 10, 2015

@rsc rsc removed release-none labels Apr 10, 2015

@bradfitz bradfitz added the Go2 label May 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment