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

Big file as request body #495

Closed
Myriad-Dreamin opened this issue Dec 4, 2021 · 11 comments
Closed

Big file as request body #495

Myriad-Dreamin opened this issue Dec 4, 2021 · 11 comments
Assignees

Comments

@Myriad-Dreamin
Copy link

I'm uploading a file as request body with size of 4GB and found that resty copies full file content into memory (to handle http.Request.GetBody callback?). However, it is not necessary because net/http.Client only views the body as an io.ReadCloser.

resty/middleware.go

Lines 521 to 543 in 7dda615

func getBodyCopy(r *Request) (*bytes.Buffer, error) {
// If r.bodyBuf present, return the copy
if r.bodyBuf != nil {
return bytes.NewBuffer(r.bodyBuf.Bytes()), nil
}
// Maybe body is `io.Reader`.
// Note: Resty user have to watchout for large body size of `io.Reader`
if r.RawRequest.Body != nil {
b, err := ioutil.ReadAll(r.RawRequest.Body)
if err != nil {
return nil, err
}
// Restore the Body
closeq(r.RawRequest.Body)
r.RawRequest.Body = ioutil.NopCloser(bytes.NewBuffer(b))
// Return the Body bytes
return bytes.NewBuffer(b), nil
}
return nil, nil
}

After reading closed issue and full code of resty, no api can help me avoid the http client reading all file content. It is expected that resty can expose a stream-like api or handle a cloneable reader implicitly to get request retrying functionality.

    client.R().SetClonableBody(reqBody.Reader)

or

    sink, err := detectBodyCloner(r) // replace getBodyCopy
    if err != nil && err != ErrBodyIsNotCloneable {
        return err
    }

    var bodyIsNotCloneable = err == ErrBodyIsNotCloneable

    // assign get body func for the underlying raw request instance
    r.RawRequest.GetBody = func() (io.ReadCloser, error) {
        if sink != nil {
            cReader := sink.Clone()
            if reader, ok := cReader.(io.Reader); ok {
                return ioutil.NopCloser(reader), nil
            }
            if reader, ok := cReader.(io.ReadCloser); ok {
                return reader, nil
            }
            return nil, ErrBodyIsNotStream
        }
        if bodyIsNotCloneable {
            return nil, ErrBodyIsNotCloneable
        }
        return nil, nil
    }

The second approach is better in the case of net/http.Client never calling GetBody callback

@kklinan
Copy link

kklinan commented Mar 9, 2022

I need it, too.
I currently do this by storing the file and then streaming the read.

@thxbb12
Copy link

thxbb12 commented Jul 26, 2022

I need the ability to upload files of many GB, thus a stream-like implementation is mandatory.
@kklinan could you please post your fix here?
Thanks!

@gospider007
Copy link

@pcfreak30
Copy link

I have just run into this and will have to jump libraries—a bit of a shame. Trying to copy an io.Reader into memory vs streaming is a really bad assumption.

I looked, and I think both https://github.com/monaco-io/request and https://github.com/imroc/req handle this correctly.

@jeevatkm
Copy link
Member

jeevatkm commented Oct 4, 2024

Done, refer to PR.

@jeevatkm jeevatkm closed this as completed Oct 4, 2024
@jeevatkm jeevatkm self-assigned this Oct 4, 2024
@thxbb12
Copy link

thxbb12 commented Oct 7, 2024

Great, thanks!
Will this PR be part of Resty's next release, v2.15.4?

@jeevatkm
Copy link
Member

jeevatkm commented Oct 7, 2024

@thxbb12 Thanks. I have added this feature as part of the upcoming Resty v3.

@thxbb12
Copy link

thxbb12 commented Oct 7, 2024

Thanks @jeevatkm. I was hoping it would be available in the next 2.15.x release.
Any ETA as to when Resty v3 will be released?

@jeevatkm
Copy link
Member

jeevatkm commented Oct 7, 2024

@thxbb12 Resty v3 will take time (possibly around the new year). It will have more features and options than v2 and be more efficient. Is this okay for your timeline?

@thxbb12
Copy link

thxbb12 commented Oct 7, 2024

Yes, that's fine. I cannot really complain as I'm not the one working on it myself (although I wish I had enough time to contribute).
Thanks for your great work!

@jeevatkm
Copy link
Member

jeevatkm commented Oct 7, 2024

Thank you @thxbb12. You can watch v3 progress here.

I will be heading to work in a while before leaving. I saw your comment. Good chat.

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

No branches or pull requests

6 participants