From 93955b755fba72e1bf2945bb84b569c8cdc1fe54 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Fri, 12 Apr 2024 12:35:39 +0100 Subject: [PATCH] operations: fix retries downloading too much data with certain backends Before this fix if more than one retry happened on a file that rclone had opened for read with a backend that uses fs.FixRangeOption then rclone would read too much data and the transfer would fail. Backends affected: - azureblob, azurefiles, b2, box, dropbox, fichier, filefabric - googlecloudstorage, hidrive, imagekit, jottacloud, koofr, netstorage - onedrive, opendrive, oracleobjectstorage, pikpak, premiumizeme - protondrive, qingstor, quatrix, s3, sharefile, sugarsync, swift - uptobox, webdav, zoho This was because rclone was emitting Range requests for the wrong data range on the second and subsequent retries. This was caused by fs.FixRangeOption modifying the options and the reopen code relying on them not being modified. This fix makes a copy of the fs.FixRangeOption in the reopen code to fix the problem. In future it might be best to change fs.FixRangeOption so it returns a new options slice. Fixes #7759 --- fs/operations/reopen.go | 2 ++ fs/operations/reopen_test.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/fs/operations/reopen.go b/fs/operations/reopen.go index b2531ced0..9b86c6da3 100644 --- a/fs/operations/reopen.go +++ b/fs/operations/reopen.go @@ -138,6 +138,8 @@ func (h *ReOpen) open() error { // Adjust range start to where we have got to h.rangeOption.Start = h.start + h.offset } + // Make a copy of the options as fs.FixRangeOption modifies them :-( + opts = append(make([]fs.OpenOption, 0, len(opts)), opts...) h.tries++ if h.tries > h.maxTries { h.err = errTooManyTries diff --git a/fs/operations/reopen_test.go b/fs/operations/reopen_test.go index 153c0334a..891888c50 100644 --- a/fs/operations/reopen_test.go +++ b/fs/operations/reopen_test.go @@ -39,6 +39,8 @@ type reOpenTestObject struct { // // This will break after reading the number of bytes in breaks func (o *reOpenTestObject) Open(ctx context.Context, options ...fs.OpenOption) (io.ReadCloser, error) { + // Lots of backends do this - make sure it works as it modifies options + fs.FixRangeOption(options, o.Size()) gotHash := false gotRange := false startPos := int64(0)