Upload chunks are buffered into a bytes.Reader and then, when transfer
accounting is active (always for a real copy), re-wrapped in accounting
before being handed to cachedReader. cachedReader only recognised a bare
*bytes.Reader, so the accounted chunk fell through to
readers.NewRepeatableReader, which copied the whole chunk again into an
append-grown slice. Every chunk (and the upload-cutoff prefix of every
file) therefore cost roughly twice its size in memory.
Look through the accounting wrapper when deciding whether the reader is
already a seekable buffer and, if so, seek the buffer underneath while
still reading through the accounting, so retries rewind without a copy.
Each chunk of a resumable upload was buffered in a fresh RepeatableReader
which grew by appending, so bulk transfers churned up to a chunk size (10
MiB by default) of heap per chunk and GC pressure.
Buffer chunks instead with multipart.NewRW from the global page pool
instead so memory is reused across uploads and bounded by rclone's
central memory management.
A source which delivers fewer bytes than its declared size now fails
before the chunk is sent with an unexpected EOF error rather than being
rejected by the transport.
Note that accounting now happens as the chunk is read into the buffer
rather than as it is sent, as in the drive backend.
When the source can't be re-opened, the whole input is read once to
compute its gcid before upload and held back for the upload proper. For
inputs at or below --pikpak-hash-memory-limit this used a bytes.Buffer,
a fresh heap allocation of up to the limit (and more while growing) per
file.
Hold the data in a buffer from the global memory pool instead so the
pages are reused and released on cleanup.
Inputs of unknown size were also always held in memory regardless of
their length, as only sizes above the limit chose the temp file.
Spool unknown sizes to the temp file so a large stream can't exhaust
memory.
The multipart uploader kept its own private buffer pool, a copy of the
one in lib/pool with identical settings, so its chunk memory was never
shared with the rest of rclone. Pages cached here were invisible to
other backends and vice versa, costing up to 64 MiB of extra idle cache.
Allocate chunks with multipart.NewRW, the global pool used by the
other backends instead.
WriteChunk copied the whole chunk, which lib/multipart already hands over
in a buffer from the global memory pool, into a bytes.Buffer so that
retries could re-send it. That doubled the per-part memory and made a
fresh chunk-sized heap allocation (64 MiB by default) for every part,
times the upload concurrency.
The chunk reader is seekable, so find its size with Seek and rewind it
inside the pacer closure instead, sending the pooled buffer directly.
Also fix the error for a part that fails to upload, which formatted the
buffer instead of the part number.
Each upload chunk is buffered in a pool.RW from the global memory pool
but was never closed, so its pages were never returned to the pool.
Close the buffer after each chunk is uploaded and on the read error
path.
A chunk that failed with a retryable error was also retried without
rewinding the buffer, so the retry sent an empty body with the original
Content-Length and Content-Range and failed.
Seek the chunk back to the start inside the pacer closure so each
attempt re-sends it in full.
The FsPutRetry integration test covers the retry of a failed upload
request and checks the buffers are returned to the pool.
When both /me/drives and /me/drive fail during config (for example an
account-level 403 serviceReadOnly "Database Is Read Only"), send the
config state machine to the existing manual drive ID entry state
instead of dead-ending at choose_type with the raw error. The drive
itself remains usable when only the enumeration API is blocked.
Fixes#9794
Each resumable upload allocated a fresh chunk-sized buffer (8 MiB by
default), so bulk transfers of many files churned allocations and GC.
Buffer chunks with multipart.NewRW instead — the global page pool used
by the other backends — so chunk memory is reused across uploads and
bounded by rclone's central memory management. The pool.RW is seekable,
which also keeps chunk reads repeatable for retries.
The pool.RW implements io.Closer, so http.NewRequestWithContext upgraded
it to the request body and the transport closed it after each attempt,
returning its pages to the global pool — a chunk retried after a 5xx
then read a freed buffer and panicked in pool.(*RW).readPage. Wrap the
request body in readers.NoCloser so the transport can't take ownership
and the upload loop remains solely responsible for the buffer's
lifetime. Add a regression test that fails a chunk with a 500 and then
accepts the retry; it reproduces the panic without the fix.
Fixes#9684
The sftp backend could be used as the source of a multi-thread copy -
each chunk of a download is read on its own connection - but not as the
destination, because it did not implement OpenWriterAt. Uploading a
single large file was therefore limited to one connection while
downloading the same file was not, capping upload throughput well below
the link speed on high latency connections.
This implements OpenWriterAt for the sftp backend using lib/filepool. A
small pool of open write handles, each backed by its own connection,
lets the core write the chunks of a large file concurrently over several
connections. The file is created and truncated once up front so every
chunk offset is valid before the concurrent writes start.
Multi-thread uploads are off by default and turned on with the new
--sftp-multithread-upload flag, since many sftp servers only accept
sequential writes and would fail large uploads otherwise. Even when
enabled they fall back to a single connection when
--sftp-disable-concurrent-writes is set (a server that can't take
out-of-order packets on one handle won't take several handles either) or
when --sftp-connections caps the pool (the per-file fan-out would
otherwise deadlock waiting on it).
The OpenSSH test server (TestSFTPOpenssh) enables
--sftp-multithread-upload so the feature is exercised in CI against a
real server.
Tested against OpenSSH: a single large upload over a high latency link
went from ~12 MB/s to ~90 MB/s.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Splainte <r.wycke@hotmail.fr>
Factor the connection-backed write handle pool out of the smb backend
into a generic lib/filepool.Pool[T] with its own tests, so it can be
reused by other backends that implement fs.OpenWriterAter over a
connection pool.
The smb backend keeps its behaviour, opening and releasing handles
through small closures passed to the pool.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Splainte <r.wycke@hotmail.fr>
Nine tests fail on an ordinary Windows machine, eight in backend/local and
TestEnvironmentVariables in cmdtest, all with
symlink file.txt \?\C:\Users\...\symlink.txt: A required privilege is not
held by the client.
Windows grants SeCreateSymbolicLinkPrivilege only to an elevated process or one
running with Developer Mode enabled, and a default install gives an ordinary
user neither. CI does not see this because the windows-latest runner is
elevated, so the failures only show up on a contributor's own machine, where
AGENTS.md asks for make quicktest to pass before opening a pull request.
cmdtest already recognised the situation and attached a note to the failure
saying the test could safely be ignored. If it is safe to ignore then the test
knows it cannot run, so skip it and say why instead.
backend/local gains a helper that tries a symlink in t.TempDir() and skips if it
cannot make one, called from the six tests that need the privilege. Where a
platform can create symlinks the probe succeeds and nothing is skipped, so other
platforms are unchanged.
TestMetadata is skipped whole because it creates its symlink before anything
else and the object built from it is used throughout.
TestSymlinkEscapeConcurrent is left alone: it goes through putLink and ignores
the error, so it never needed the privilege.
Directory names which look like they have a --b2-versions version
string are now encrypted in full, so directories created by older
rclone (which left the version string in plain text) no longer
decrypt and vanished silently from listings.
DecryptDirName now falls back to the old form for such names so the
directory is listed, and logs the name it needs to be renamed to on
the underlying remote to make it accessible again. Document this in
the crypt docs.
The refresh endpoint returns a rotated token with a fresh expiry on
every successful call, but getUserInfo discarded it, so routine use
never extended the stored token's life. Once the stored token aged
out, accounts with 2FA enabled could not recover non-interactively
and required a manual reconnect.
Carry the rotated token out of getUserInfo and persist it in NewFs
via the same jwtToOAuth2Token + oauthutil.PutToken path that
refreshJWTToken uses, keeping f.cfg.Token in sync (same pattern as
refreshOrReLogin).
Fixes#9584
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The --b2-versions support added in 3fe2aaf96 strips a version string
from the last segment of a path before encrypting it, so that the
plain text version suffixes which the underlying backend appends to
encrypted file leaf names can be handled. EncryptDirName and
DecryptDirName share that code, so the last segment of a *directory*
name was version stripped too. Only file leaf names are ever given a
version string by the backend - a directory gets a
version-string-like name from the user, and such a name is encrypted
verbatim when it appears as the parent of a file name, so the same
directory ended up with two different encryptions.
Before this change, with a directory whose name matches rclone's
version format, eg dir-v2001-02-03-040506-123:
rclone copy file.txt crypt:dir-v2001-02-03-040506-123/
rclone ls crypt:dir-v2001-02-03-040506-123
# => "directory not found" - the file is invisible to listings
rclone mkdir crypt:dir-v2001-02-03-040506-123
# => creates a second directory with the same decrypted name
After this change EncryptDirName and DecryptDirName encrypt directory
names verbatim, so a directory encrypts the same way whether it is
named on its own or as the parent of a file. Version strings are only
added to file names by the underlying backend, so --b2-versions is
unaffected and the existing version tests are untouched.
A directory which was created by the old EncryptDirName will no longer
decrypt and will be reported as undecryptable in listings. Such
directories were already unusable - anything copied into one was
written to a different encrypted directory - so nothing which worked
before is broken by this.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With no_head_object set, NewObject does not read any metadata, so the
destination object returned from a server side copy had a size of 0.
The size check in operations.Copy then failed with "corrupted on
transfer: sizes differ N vs 0" and deleted the newly copied object.
This also broke Move and hence renames through rclone mount.
Populate the destination object's size and MD5 from the source object
when no_head_object is set, as a server side copy produces an object
with identical content.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dropbox is case insensitive and the path_display it returns in
change notifications may not match the case of the configured root.
Before this change the root was trimmed with a case sensitive prefix
match, so when the cases differed the full path was passed to the
ChangeNotify callback and the notification was ignored.
This trims the root case insensitively while preserving the display
case of the remaining path.
A successful UploadPart whose response carries no ETag header made
WriteChunk panic dereferencing uout.ETag in a debug log line. The part
ETag is required by CompleteMultipartUpload, so an ETag-less 200 is
unusable: return a retryable error from inside the pacer callback so
the chunk is retried instead of crashing the transfer or completing
the upload with a broken part list.
Fixes#9822
Co-authored-by: Shurong Cao <170531907+CAOShurong@users.noreply.github.com>
Nextcloud only stores a checksum which is supplied in the OC-Checksum
header of an upload, and discards it again when the modification time
is set with PROPPATCH. Re-sending the checksum in the PROPPATCH (as is
done for ownCloud) is rejected by Nextcloud with 403 Forbidden which
made the whole PROPPATCH fail, so SetModTime returned an error on any
object which had a hash. Uploads from sources without hashes, eg
streamed uploads with `rclone rcat`, were stored with no hash at all.
Use the Nextcloud PATCH extension with the X-Recalculate-Hash header
to have the server calculate and store the SHA1 of an object after a
streamed upload and after setting the modification time. This gives
a server side hash of the stored data which also lets rclone verify
streamed uploads.
If the source supplied fewer bytes than its declared size, the single
part upload path accepted the short body and stored a truncated file
recorded with the declared size, reporting a successful upload. The
multipart path already checks for this.
Count the bytes actually read and fail the upload if they do not match
the declared size, which cancels the partially created file.
This was found by the FsPutShortEOF integration test.
listSharedFolders already decoded shared-folder names with
f.opt.Enc.ToStandardName, but listReceivedFiles stored the raw name
returned by the Dropbox API unchanged. Names that require encoding
(e.g. a trailing space, which Dropbox itself rejects, so rclone
stores it as "name␠" via EncodeRightSpace) were therefore shown under
their raw, encoded form for received files instead of being decoded
back to the standard name, and findSharedFile could not resolve such
a file by its standard name.
Apply the same ToStandardName conversion listSharedFolders uses.
On a ranged download the metadata decoder stored the response's
Content-Length (the length of the range, not the blob) in the object's
size and only corrected it from the Content-Range total afterwards.
Object.Size() is read concurrently by the VFS cache and chunked reader
while a download is in progress, so with --vfs-read-chunk-size a reader
could observe the chunk length (e.g. 67108864 for 64M chunks) as the
object size. The VFS cache then logged
vfs cache: cached file (N) is unexpectedly larger than the remote
object (67108864). The cached file is likely corrupted after an
unclean shutdown; recovering ...
and truncated the read request against the bogus size, breaking
sequential reads of large blobs with --vfs-cache-mode full.
This applies the Content-Range correction before the size is stored so
the range length is never published as the object size.
The multipart upload copied the source into the request buffer without
checking how many bytes it had read, so a source that supplied fewer
bytes than its declared size was accepted by the server and reported as
a success with a truncated file stored.
Count the bytes actually read and fail the upload if they do not match
the declared size.
Signed-off-by: Rohit Behera <126186063+r0h1tb@users.noreply.github.com>
The single-shot upload path sent the source straight to Box as a multipart
body with no Content-Length, so a source that supplied fewer bytes than its
declared size produced a short request that Box accepted and stored, and the
upload was reported as a success.
Count the bytes actually read and fail the upload if they do not match the
declared size. The multipart path already reads each chunk with io.ReadFull
and so already fails in this case.
The concurrent walker created by walk() only stopped when the callback
returned an error or the whole tree had been listed. Cancelling the
context (for example via the rc job/stop endpoint for an async
operations/size or recursive operations/list call) was therefore
ignored: the checkers kept pulling list jobs from the channel and kept
listing the entire tree, burning CPU and making job cancellation
useless for every backend without a native ListR implementation.
Make every checker select on ctx.Done() so a cancelled walk shuts down
promptly through the existing quit/drain path and reports the context
error. Also check the context between directory read chunks in the
local backend so a single huge directory does not block cancellation.
Update already wrapped the source in a counting reader but never looked at the
count, so a source that supplied fewer bytes than its declared size was
uploaded as a chunked request, accepted by the server and reported as a
success with a truncated file stored.
Compare the bytes actually read against the declared size.
When copying to the local backend with checksums enabled (the default),
rclone hashed the incoming data by wrapping the source reader in an
io.TeeReader. TeeReader has no WriteTo method, so io.Copy could not use
the source's fast path and fell back to its generic 32 KiB buffer loop.
The same wrapping also stopped the destination *os.File using
copy_file_range, since the source was no longer a raw fd.
This meant checksummed copies were written in 32 KiB chunks whereas
--ignore-checksum copies were written in much larger blocks (typically
1 MiB). On filesystems where small writes are expensive, such as FUSE
mounts like LucidLink, this made a big difference: copying a 100 MiB
file took 3203 x 32 KiB writes in 3.6s, and now takes 108 x ~1 MiB
writes in 0.47s.
When a multi-thread source ignores ranged reads, abort the partial
destination and retry the copy as a single stream. Reset accounting
before the fallback so transfer progress remains accurate.
Validate ranged GET responses before returning their bodies. Close invalid
responses, retry malformed partial responses through the pacer, and return
without retrying when a server deterministically ignores Range.
Fixes#6980
Before this change, when no_data_encryption was set, uploads from
local disk advertised the hash of the encrypted data even though the
data was uploaded unencrypted.
On backends which check upload hashes (eg b2) this made uploads of
small files fail with errors like "Checksum did not match data
received", and made chunked uploads store an incorrect hash so the
files failed their checksum on download with "corrupted on transfer:
SHA1 hashes differ".
See: https://forum.rclone.org/t/sha1-mismatches-on-b2-with-no-data-encryption-true/54121
Nick Craig-Wood
SillyZirandNick Craig-Wood
Mikel Olasagasti UrangaandNick Craig-Wood
DhevenddraandNick Craig-Wood
Loi NguyenandNick Craig-Wood
waterandNick Craig-Wood
machsixandNick Craig-Wood
Sanjay Kanth AandNick Craig-Wood
kingston125andNick Craig-Wood
Rohit BeheraandNick Craig-Wood
Rahman YilmazandGitHub
Dominik SanderandGitHub
MoraxandNick Craig-Wood