bufio.NewReader and http.NewRequest

457 views
Skip to first unread message

Thomas Bruyelle

unread,
Nov 7, 2013, 4:51:00 PM11/7/13
to golan...@googlegroups.com
Hi all,

I use github.com/google/go-github library to upload assets to github Release. Internally the upload is done with http.NewRequest with the Reader passed in parameter.

I manage to make it work without issue by passing the Reader as a bytes.newReader(ioutil.ReadFile(assetPath)) (the code is intentionally compressed). 
But this way to do can be a problem when dealing with large files. So I tried to pass a Reader as bufio.NewReader(os.Open(assetPath)), but in this case the upload fails with error 400 Bad Content-Length:  []

I looked at the source and it seems bufio.Reader type is not handled by the http.NewRequest function. I guess it's because it's not aware of the file size at this step, no?

How I pass a buffered Reader to http.NewRequest ?

Dave Cheney

unread,
Nov 7, 2013, 5:00:36 PM11/7/13
to Thomas Bruyelle, golang-nuts
> bytes.newReader(ioutil.ReadFile(assetPath))

Would not compile as it returns two values. Can you please post a code sample.
> --
> You received this message because you are subscribed to the Google Groups
> "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-nuts...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.

DisposaBoy

unread,
Nov 8, 2013, 2:45:50 AM11/8/13
to golan...@googlegroups.com
Am I missing something? why not open the file and pass that instead of trying to read it into memory

Thomas Bruyelle

unread,
Nov 8, 2013, 4:12:16 AM11/8/13
to golan...@googlegroups.com
> Am I missing something? why not open the file and pass that instead of trying to read it into memory

I have the same error when passing the file.

You can find full code here https://github.com/tbruyelle/gostuff

But to run it you have to declare a github repository and a access token. Also the repo needs at least one Release.



Carlos Castillo

unread,
Nov 8, 2013, 4:43:54 AM11/8/13
to golan...@googlegroups.com
The problem is that RepositoryService.UploadReleaseAsset doesn't let you pass in a size to complement the io.Reader. The http.NewRequest function (eventually called by the go-github package) can only automatically determine the size of strings.Readers, bytes.Readers, and bytes.Buffers, so if you were using the http library raw, you would have to call NewRequest, set the ContentLength field on the http.Request yourself, and then use the Do method to perform the upload. Since the github library method you're using doesn't let you work this way (set a ContentLength before the request executes) you have a few options:

 1. Do as you have been doing, load the file into a byte slice and pass a bytes.Reader, since the ContentLength can be determined automatically by http.NewRequest
 2. Use MMap to get the byte slice and then do the rest of #1 (better memory use)
 3. Copy the logic in the RepositoryService.UploadReleaseAsset method (and addOptions) into your own function, and then determine the file size yourself to set the ContentLength of the http.Request before the upload
 4. Lobby go-github to add a filesize argument to UploadReleaseAsset (or provide some other method of solving this problem)
 5. Lobby go to update the logic of http.NewRequest to allow custom io.Reader's to provide the size for content-length. (If accepted, would probably have to wait until after go-1.2 is released.)
 6. Lobby github to change the api to allow file uploads without a Content-Length (very unlikely to happen)

Carlos Castillo

unread,
Nov 8, 2013, 5:13:15 AM11/8/13
to golan...@googlegroups.com
I have started the process going for #5 by submitting https://code.google.com/p/go/issues/detail?id=6738

Thomas Bruyelle

unread,
Nov 8, 2013, 8:38:16 AM11/8/13
to golan...@googlegroups.com
Awesome answer, both very instructive and funny (I laughed at #6). I love the Go community :)

Actually I'm the author of the RepositoryService.UploadReleaseAsset, and I was trying to improve it. So I'll probably move forward with solution #4. 
Now I have a clear understanding of the problem, thanks!

I also voted for #5.

Edward Muller

unread,
Nov 13, 2013, 4:37:34 PM11/13/13
to Thomas Bruyelle, golan...@googlegroups.com
I've argued that the go http client should cast the reader to an interface that provides Length() int or something similar (ContentLength() int?) in the past and still think this is the most effective way to handle this.
--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


--
Edward Muller
@freeformz

Dave Cheney

unread,
Nov 13, 2013, 4:41:47 PM11/13/13
to Edward Muller, Thomas Bruyelle, golan...@googlegroups.com
Have you raised an issue to that effect?
Reply all
Reply to author
Forward
0 new messages