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

Use seeker when available on request body #50

Merged
merged 1 commit into from Nov 8, 2021
Merged

Use seeker when available on request body #50

merged 1 commit into from Nov 8, 2021

Conversation

zekroTJA
Copy link
Member

@zekroTJA zekroTJA commented Nov 8, 2021

Problem

I tried to upload a large file (~30GB) over night with on my Raspberry Pi using the CLI client. Therefore I opened up a Docker container with the binary and the file to upload inside and started the upload from there. I noticed that the upload always gets canceled after a few minutes with log output Killed.

The problem occurs because the contents of the file are written into a buffer on upload, which filled up the RAM of the Pi (4GB) which eventually caused Linux to kill the process.

gowebdav/requests.go

Lines 12 to 16 in 73a7f0b

func (c *Client) req(method, path string, body io.Reader, intercept func(*http.Request)) (req *http.Response, err error) {
// Tee the body, because if authorization fails we will need to read from it again.
var r *http.Request
var ba bytes.Buffer
bb := io.TeeReader(body, &ba)

I was also able to reproduce this on my Windows system.

Solution

I changed the req function so that it checks if the passed body Reader implements io.Seeker. If this is the case, the streams cursor will be reset to start before request which eliminates the requirement of the retry buffer. Otherwise, if the Reader is not seekable, the stream will be teed into the retry buffer.

Also, I've renamed the buffer ab to retryBuf for better understanding.

@chripo
Copy link
Member

chripo commented Nov 8, 2021

link #24 and #15
thank you!!
this is a more or less know issue and the whole mess is related to authorization handling in some servers.
my initial idea was to refactor this out into strategies and on top one that handles it without fuss. still a TO DO. :)

@chripo chripo merged commit 2f2cda4 into master Nov 8, 2021
@chripo chripo deleted the develop branch November 8, 2021 08:55
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

Successfully merging this pull request may close these issues.

memleak during multiple file upload when authentication is required improve authentication
2 participants