CopyObject with a directory marker such as "dir/" as its source failed
with InvalidArgument, although HeadObject and GetObject found it.
It now copies the marker, making the directory of another marker or an
empty object.
See #10082
Putting a directory marker such as "dir/" made the directory, but a
listing without a delimiter didn't return the "dir/" key, so clients
which make empty directories with markers, as the AWS console and many
sync tools do, lost them.
An empty directory is now listed as its directory marker. Other
directories aren't, as they may not have been made by putting one.
See #10082
With an --etag-hash other than MD5, including "" to turn hashing off,
PutObject returned the MD5 of the data and CompleteMultipartUpload an
S3 style multipart ETag, neither of which matched the ETag HeadObject
and listings returned for the object afterwards.
These now return the ETag HeadObject does. Listings now quote an empty
ETag as HeadObject does.
See #10082
This updates expected-pass.txt for the fixes to gofakes3 and serve
s3, which make 125 more of the s3-tests pass, 297 in all.
36 tests no longer pass, as they only passed because requests were
silently ignored or mishandled:
- 19 used SSE-C or SSE-KMS, now refused as they aren't supported
- 12 browser POST uploads expected 400, but unauthenticated requests
now get 403 as on AWS
- 5 set an ACL other than private, or object lock, now refused
The s3-tests config now gives the owner gofakes3 uses for the main
user, so the ACLs returned match it, and no api_name, as bucket
locations aren't stored.
gofakes3 now checks the If-Match and If-None-Match conditions of
PutObject and CompleteMultipartUpload before the object is written,
but another write can replace the object before this one finishes.
For example, of many concurrent uploads of a new object with
If-None-Match: *, most succeeded.
Now when the object is written to a temporary object and moved into
place, the conditions are checked again as it is moved, with the moves
of objects to the same path through the server serialised, so only one
of the uploads in the example succeeds.
CompleteMultipartUpload returned the AWS S3 style multipart ETag (the
MD5 of the parts' MD5s with a "-N" suffix), but HeadObject and GET
returned the hash of the object, so clients saw the ETag of an object
change as soon as it was uploaded.
Now with --etag-hash MD5, the default, the ETag returned by
CompleteMultipartUpload is the MD5 of the whole object, worked out as
it is streamed, so is the same as HeadObject returns. With other
hashes it is stored with the object's metadata and returned by
HeadObject while the metadata is kept.
The sizes of the parts are kept with the metadata too, so GET and
HEAD with partNumber and GetObjectAttributes can return the parts of
the object.
Keys ending in "/", such as "dir/", are used by S3 clients as
directory markers. gofakes3 used to strip the "/", so putting "dir/"
made a file "dir", which then stopped "dir/file" being written. Now
that gofakes3 keeps the "/" these keys were refused.
Now a key ending in "/" stands for the directory: putting an empty one
makes the directory, putting one with data is refused, HEAD and GET
return an empty object if the directory exists, and deleting one
removes the directory if it is empty.
Writing an object which conflicts with a directory, or whose directory
conflicts with an object, now fails with InvalidRequest rather than an
internal error, which clients retried.
Listings treated any delimiter as "/", so a listing with a delimiter
such as "a" or "." returned the keys and directories rather than the
keys rolled up into common prefixes at the delimiter. A delimiter of
whitespace was ignored altogether.
Now any delimiter is supported. Delimiters other than "/" are found by
walking the directory tree below the prefix, skipping directories
whose names contain the delimiter as all their keys roll up into the
same common prefix.
A prefix starting with "/" now matches nothing, as no key can start
with "/", rather than listing the whole bucket.
The ETags of objects in listings were returned without the double
quotes S3 uses, as in HEAD and GET responses, so clients which compare
them saw a difference.
Object tags given when an object is created are kept with its metadata
by gofakes3. This implements gofakes3's ObjectTaggingBackend so the
tags of an existing object can be changed with PutObjectTagging and
DeleteObjectTagging.
Like the rest of the metadata, tags are only kept in memory.
CopyObject merged the source object's metadata into the metadata it
was given, so a copy with x-amz-metadata-directive: REPLACE kept the
source's metadata. It also kept the source's modification time by
adding it to the metadata, so every copy gained "mtime" user metadata
the client hadn't set.
gofakes3 now works out the complete metadata of the copy from the
metadata directive, so it is used as given. The source's modification
time is still kept, unless the metadata sets one, but without adding
it to the metadata.
Starting a multipart upload makes the directory its object will be in,
but aborting the upload didn't remove it. The empty directory isn't
visible to S3 clients, so the bucket looked empty, but deleting it
failed with BucketNotEmpty.
Now aborting an upload removes the directories made for it if they are
still empty, and deleting a bucket holding nothing but empty
directories removes them and succeeds. Deleting a bucket which fails
for some other reason than holding objects now returns that error
rather than BucketNotEmpty.
Removing the empty directories left after deleting an object now
checks they are empty first, rather than logging an error for each
directory which isn't.
This adds cmd/serve/s3/s3tests/run.sh which runs the ceph/s3-tests S3
compatibility suite against rclone serve s3, built from the tree.
It fetches s3-tests at a pinned commit, makes a venv with pinned
dependencies, starts rclone serve s3 with the users s3-tests needs and
summarises the results by feature area.
As serve s3 doesn't implement many S3 features (ACLs, versioning,
lifecycle, policies, etc) the results are checked against
expected-pass.txt, a list of the tests which should pass, rather than
a list of failures. The run fails if any of these don't pass, and
run.sh --update records newly passing tests.
A small pytest plugin adapts the s3-tests cleanup to serve s3 so a
bucket which can't be deleted fails only the test which left it
rather than every test after it.
Some S3 providers, such as Wasabi and Alibaba, report success when
creating a bucket which already exists, so rclone logs that the bucket
was created. The during-sync hooks in the equal_skipped test create a
new Fs, which logged this in the middle of the bisync run and made the
log differ from the golden one. Ignore these messages.
If a file is changed on both sides and the conflict names it gets renamed
to (e.g. X.conflict1 and X.conflict2) were deleted on both sides since the
last run, the listing update removed the just-copied conflict files again
when processing the deleted-on-both queue. The rename bookkeeping then
could not find them and bisync aborted with "bisync.renames: internal
error: missing info".
Skip names that this run copied when removing files deleted on both sides,
and add a test case for the scenario.
gofakes3 adds the time of the request as "Last-Modified" to the
metadata it passes to PutObject, CopyObject and multipart uploads.
storeMeta cached it with the rest, so HeadObject and GetObject
replaced the object's modification time with the upload time until the
cache entry was evicted, while ListObjects reported the right time.
storeMeta now drops "Last-Modified" before caching the metadata.
Before this change, `cmd/bisync/testdata/bisync_vscode_debuggers_launch.json`
was in the repository, and every `-golden` run regenerated it from
`fstest/test_all/config.yaml`, adding unnecessary noise to commit diffs.
After this change, the file is in `.gitignore` and no longer tracked.
`-golden` still writes it locally, for copying debug configurations into
VS Code's `launch.json`.
Before this change, the `{eol}`, `{tab}` and `{chr:XX}` placeholders in a
scenario became raw bytes in the name the harness handed to `Put`. A raw
control character isn't a valid rclone name: rclone names are in the
`Standard` encoding, where a newline is `␊`, and `FsEncoding` in
`fstests` encodes every raw name before uploading it for the same
reason.
The `extended_filenames` scenario dates from 2021, when bisync was tested
on local, Drive, Dropbox, OneDrive, S3 and SFTP, but not on crypt or
memory. Its header said that all rclone backends, including local,
return `\n` as a glyph. That was an assumption, and it doesn't hold. A
backend with an encoder lists a raw control character as its glyph
because `ToStandardName` runs `Standard.Encode` on every name it
returns. A backend without one (crypt, memory, linkbox, netstorage,
pixeldrain, storj) lists exactly the name it was given. The same thing
happens with no bisync involved: `rclone touch` with a raw newline in
the name stores a raw newline on crypt, and `sync` from there to local
then copies and deletes the file on alternate runs.
The scenario has never passed on crypt or memory for the right reason.
Its golden listings were recorded on local with `␊`, so until January
2024 they would not have matched the raw names crypt lists, but the
scenario wasn't run there. `57624629d` ran it on crypt, treated the raw
names as an encoding difference, and normalized them away in both the
log and the listings, so from then on the goldens matched. `81d7b6f29`
added the check that each listing matches its path, which that
normalization doesn't cover, and the step has failed on those backends
since: local lists the copy as `␊` while the listing, built from the
source names, holds the raw newline. The pre-flight probe fixed in the
previous commit skipped the scenario on many nightly backends in the
meantime, which kept the failure out of the nightly results.
After this change, a placeholder for a control character (0x01-0x1f and
0x7f) becomes its `Standard` glyph, so every backend receives the same
name. On local the file on disk is unchanged, since the local backend
decodes the glyph to the raw character, and the golden logs are
unchanged. Placeholders for other bytes, such as the invalid UTF-8 in
`{chr:81}` and `{chr:fe}`, still become raw bytes, which `Standard`
leaves as they are.
Before this change, the pre-flight check for the `normalization` and
`extended_filenames` cases uploaded its probe file to the path root, which
already held the scenario's files, and compared the first entry each path
listed. That entry was usually an unrelated file, so the result depended on
listing order rather than on how each backend stored the probe name. On 42
of 71 backends in the 2026-10-15 nightly, the paths appeared to differ and
the cases were skipped; elsewhere the check passed without examining the
probe at all. The check also named `extended_char_paths`, a case renamed to
`ext_paths` in 2024. The probe's uploads counted in the step's transfer
stats, which removed `There was nothing to transfer` lines from the logs of
the checked cases.
After this change, the probe goes in a fresh subdirectory with its own
stats group, and the check compares the name listed back from there. It
names only `normalization` and `extended_filenames`: `ext_paths` uses none
of the probe's invalid UTF-8 or emoji, and doesn't use `--fix-case`. The
`normalization` and `extended_filenames` golden logs gain the lines the
probe's stats had hidden.
deleteObject() replied 404 for any error from newObject(), while
serveObject() in the same file replies 404 only for
fs.ErrorObjectNotFound and 500 otherwise.
restic treats a 404 on DELETE as final and never retries, so a
transient lookup failure while removing a lock left the lock behind
and broke the next exclusive operation (eg forget --prune failing with
"repository is already locked").
This change mirrors serveObject() handling of 404/500 errors in
deleteObject().
Fixes#10100
Co-authored-by: bunnysayzz <bunnysayzz@users.noreply.github.com>
The tcell v2 pin at v2.9.0 is removed as tcell v3 contains the fix for
duplicated key events on Windows (tcell issue 1124).
Note that tcell v3 uses the Windows VT console modes so ncdu requires
Windows 10 build 1703 or later.
See: #9539
This fixes the code to compile with the updated dependencies:
- dlna: pass a *slog.Logger to the ssdp server as required by
anacrolix/dms v1.8.0
github.com/Files-com/files-sdk-go/v3 was not updated as the latest
version requires go 1.26.9.
The tcell and go-systemd pins are kept and github.com/hanwen/go-fuse/v2
is pinned to v2.11.0 as v2.12.0 fails to compile on 32 bit linux - see
https://github.com/hanwen/go-fuse/issues/628
Before this change, the integration tests dropped Dropbox's "Too many
requests or write operations" notice from the logs only in the format it
had when the tests were written, which began with the error code
(`too_many_requests/...`). Since 110bf468a (v1.65), the notice begins with
`Error `, so it was left in the log and caused a miscompare whenever
Dropbox rate limited a run.
After this change, the notice is dropped in either format.
Before this change, the integration tests checked whether each remote could
set modtimes on every `bisync` step (`checkPreReqs`, called from
`runBisync`), not once per remote. On a remote that isn't `IsLocal`, each
check sleeps for 1s twice (the Google Cloud Storage per-object rate limit
workaround from be73a10a9), for each side. That added 2-4s to each of the
~110 `bisync` steps in each test, which on a fast remote was most of the
run: on the integration test server, the `:memory:` tests took about 14
minutes.
After this change, the result of the check is remembered for each remote,
so it runs, and sleeps, once per remote per test run. The `:memory:` tests
now take about 7s locally.
Before this change, nothing checked that the rc `sync/bisync` call accepted
every bisync flag, so the rc could fall behind the command line as flags
were added, as in #7799.
After this change, `TestRcFlagParity` passes each non-hidden bisync flag to
the rc, under the parameter name `rc.md` uses, with a value of the wrong
type. Every getter `rcBisync` uses rejects it, so the call fails only if
the parameter is read, and the test names any flag the rc doesn't handle.
Before this change, since v1.74.0 (bb78eb8ab), the rc `sync/bisync` call
read the `backupdir1` and `backupdir2` parameters (the names used before
they were renamed to `backupDir1` and `backupDir2`) only when the new name
was also given with an invalid value, because the fallback sat inside an
`rc.NotErrParamNotFound(err)` check, which is true only for an invalid
value. Otherwise, `backupdir1` and `backupdir2` had no effect.
The same change made an invalid value for any optional parameter (for
example `"resync": "yes"`, or a `filtersFile` that isn't a string) be
treated as if the parameter were missing, instead of returning an error.
After this change, the old backup dir names are read when the new ones are
missing, and an invalid value returns an error again, as it did before
v1.74.0.
Before this change, bisync treated a `--max-delete` above 100 as 100%, so
the `--max-delete` check never triggered. A run that left no file on a side
unchanged was still stopped, by the "all files were changed" or empty
listing checks, but one that deleted most files while leaving even one
unchanged was not.
For bisync, `--max-delete` has always been a
percentage, "in range 0..100" as Ivan put it when he designed it
(https://github.com/rclone/rclone/pull/5164#discussion_r633470815), but the
range was never enforced. So a value meant as a file count, such as
`--max-delete 500`, turned the check off. Now that the rc takes
`--max-delete` from the global config too, that includes a `--max-delete`
the rcd itself was started with.
After this change, a `--max-delete` above 100 is an error, as the rc's
`maxDelete` parameter already was.
Before this change, `--max-delete` and `--dry-run` were read from the
global config by `applyContext`, which only the command line called. The
rc `sync/bisync` call started from empty `Options` and never ran it, so it
ignored both settings:
- A dry run requested through the global option (`_config`
`{"DryRun": true}` or the flat `dry_run` parameter) was ignored.
`setDryRun` resets `ci.DryRun` from `opt.DryRun` before copying, and
`opt.DryRun` was only set by bisync's own `dryRun` parameter.
- When the `maxDelete` parameter was omitted, the `--max-delete` limit was
0%. A `max_delete` set through `_config` or the flat parameter was not
used for that limit, and applied to the inner syncs as a file count
instead.
After this change, `Bisync` runs `applyContext` itself, on its own copy of
the config, so the command line, the rc, and any other caller follow the
same rules, and `Options.MaxDelete` and `Options.DryRun` are set by
`Bisync`. With no `--max-delete` anywhere, the limit is 50%, as #5680 says
the rc should.
The rc's `maxDelete` and `dryRun` parameters now set the call's config.
`maxDelete` overrides the global `--max-delete`. `dryRun: true` asks for a
dry run, but `dryRun: false` doesn't cancel one from the global config, so
the run is a dry run if either asks for one. The `maxDelete` parameter is
also documented again; it was left out when the rc help became generated
from the command's flags.
Before this change, when bisync updated a listing entry, it kept the old
modtime if the new one was less than a second earlier. This dated back to
when bisync truncated modtimes to the destination's precision before
recording them (`ConvertPrecision`), so that a truncated time would not
overwrite the more precise original.
`ConvertPrecision` was removed in 956c2963f, and since then, bisync has kept
the original precision and relied on the modify window when comparing. But
the rule in `ls.put` was left behind, and it then fired only when a file
really had an earlier modtime, recording a time the file did not have. This
could cause:
- a `--resync` to abort with "path1 and path2 are out of sync" when Path1's
version was slightly older than Path2's
- later runs to report changes that didn't happen, for a file changed to a
version less than a second older, or copied from a remote with lower
modtime precision within the same second as the old version (for example,
from SFTP to local)
After this change, bisync records the modtime it is given, like any other
update. Comparisons already account for differences in precision through
the modify window.
- TestCheckConflictsSkipsCheck: the check for identical files is skipped
with --size-only, and with --ignore-checksum only when the paths have no
hash in common.
- test_equal_skipped: an identical file that is new on both paths with
--ignore-checksum is recorded, so deleting it on Path1 deletes it on
Path2. Adds the "ignore-checksum" scenario argument.
- test_conflict_window: Path1 changes between the listing snapshot and
the check for identical files.
Co-Authored-By: ASRodrigo1 <45823308+asrodrigo1@users.noreply.github.com>
Before renaming a sync conflict, bisync checks whether the two files are
identical. It compares hashes using check or cryptcheck, and as a last
resort, when neither is available, it compares the files by downloading
them (--download), so that the result is certain regardless of checksum
support.
Before this change, the fallback to --download happened only when bisync
knew up front that the remotes had no usable hash. It did not happen
when a hash that was expected to be present turned out to be missing
(for example, a backend that normally supports hashes returned a blank
one, or cryptcheck could not get the underlying hash). Those files were
counted as identical based on size alone.
After this change, a missing hash falls back to --download, the same as
when the remotes have no hash in common. --size-only and
--ignore-checksum still disable the fallback and keep the size-only
comparison.
Co-Authored-By: ASRodrigo1 <45823308+asrodrigo1@users.noreply.github.com>
Before this change, when a file was new or changed on both paths and the pre-sync
conflict check deemed the two copies identical, bisync skipped the copy and then
rechecked the file after the sync to get its listing info. If the recheck could
not confirm equality (for example, hashes unavailable due to --size-only or
(--ignore-checksum + no common hash)), the file was rolled back to be retried on
the next run, and every later run repeated the same check with the same result.
A later change on one side was then handled against a missing or stale listing
entry.
After this change, files we've declared identical pre-sync are no longer
rechecked post-sync. The pre-sync conflict check now also returns the values it
compared, and a skipped file is recorded from the listing snapshot. If either
path changed between the snapshot and the check, the prior listing is kept and
the next run rechecks.
When --size-only or (--ignore-checksum + no common hash) are in use, the "new or
changed on both paths and equal" case skips the conflict check entirely, because
it is unnecessary. The check could only compare sizes, which we already know are
equal from the snapshot.
The new `test_equal_skipped`, `test_conflict_window`, and
`test_conflict_window_size` scenarios cover identical files that change on
one path during the sync, files that change between the listing snapshot and
the check for identical files, and `--size-only` files with the same size but
different content.
Co-Authored-By: ASRodrigo1 <45823308+asrodrigo1@users.noreply.github.com>
Adds a test-only TestFnConflictCheck option, called before checking
potential conflicts, and a "hook <when> <step>" scenario command that runs
an ordinary scenario step once when the next bisync reaches that point:
before-check (TestFnConflictCheck) or during-sync (TestFn). A hook that
never runs is noted in the log so a scenario can't silently skip its setup.
On shutdown the batcher logged "Committing uploads - please wait..."
but nothing once the commit had completed, so the log looked as if
rclone had stopped part way through. Log "Committing uploads - done"
when the pending batches have been committed.
This affects the backends which use the batcher, dropbox and
googlephotos.
This change adds a step to the bisync tests that checks that after a successful
run, the files named in each listing are exactly the files on that path. This
ensures that nothing is silently missing from the listings.
Before this change, when an existing file had changed on both paths, the
conflict winner was chosen using only the attributes that had changed on each
side. An attribute that hadn't changed was treated as zero, so --conflict-resolve
larger could pick the smaller file (for example, when one side only had its
modtime updated), and --conflict-resolve newer could fail to pick a winner when one
side's modtime hadn't changed.
After this change, the current size, modtime, and checksum (to the extent
enabled with compare settings) are now recorded for every changed file.
Before this change, when --conflict-resolve picked Path2 as the winner, the winning
file was copied to Path1 correctly but then removed from both listings while
updating them for the renamed (or deleted) Path1 copy. The next run saw the file
as new on both paths and had to check it again, and a change to it on one path
in that run was handled as if the file had never been synced. When Path1 won,
the listings happened to be updated in an order that avoided this.
This change fixes the issue by adding a guard to remove the file from the
listing only when dstNewName != dstOldName.
See #9939
golang.org/x/net v0.60.0 deprecates its HTTP/2 server and transport in
favour of net/http.
Make the h2c test use http.Transport with unencrypted HTTP/2. serve
restic --stdio still needs http2.Server to serve HTTP/2 on a single
connection as http.Server has no way of doing that.
The metadata of the objects of all users was kept in one store, so with
--auth-proxy or several --auth-key credentials one user storing the
metadata of many objects could make the server forget that of every
other user.
Each user (each access key) now has a store of their own.
--multipart-max-uploads and --multipart-streaming-buffer-total limited
the uploads of all users together, so with --auth-proxy or several
--auth-key credentials one user could use up the limits and stop every
other user uploading.
They now limit the uploads of each user (each access key) separately.
A multipart part which arrives before the parts ahead of it is buffered
and acknowledged to the client, then written to the backend when its
turn comes. If that write failed the part was dropped, possibly with
some of it already written, but the upload carried on as if nothing
had happened, so it could be completed with the part missing, or with
a resend of the part appended to the partial data.
Such an upload is now failed, so the client starts it again.
The SigV4 verification in gofakes3 only checked the headers the
client listed as signed and never looked at the other headers which
arrived. A presigned PUT URL signs only the host header, so a holder
of one could add an unsigned x-amz-copy-source header and have the
server copy any object it can reach - in any bucket, and in any
remote when serving a cloud backend - into the object the URL names,
then read it back with the same URL. This affected both --auth-key
and --auth-proxy. AWS refuses such a request with AccessDenied, as
it requires every x-amz-* header to be signed.
This updates gofakes3 to a version which refuses requests carrying
an x-amz-* header not in their signed headers list with AccessDenied,
and adds tests that a presigned PUT URL can't be turned into a copy
in either auth mode. Real S3 clients always sign x-amz-* headers so
they are unaffected.
In v1.75 the multipart reorder buffer admitted the next part the stream
needed however big, adding its client-declared length to the running
total unchecked. Two concurrent requests for the next part declaring
huge lengths overflowed the total negative, after which out-of-order
parts were admitted past --multipart-streaming-buffer-limit.
The next part is now streamed straight to the backend without being
charged to the buffer, and a buffered part is only admitted if it fits
in what is left of the limits, so this can no longer happen. Add a
test of the attack against both the per-upload and server-wide limits.
serve s3 keeps the metadata of objects uploaded through it in memory.
If the object was then changed some other way - by another --auth-proxy
user sharing the backend, another server or directly on the backend -
serve s3 went on returning the old metadata, including the old
modification time, for the new object.
The size and modification time of the object are now stored with its
metadata, which is ignored if they no longer match.
With --auth-proxy the backend of each user is shut down once it has
been unused for 5 minutes. A request didn't count as using it, so a
long upload or download could have its backend shut down under it and
fail. A multipart upload used the backend it started on throughout, so
failed if the client paused for more than 5 minutes between parts, or
the auth proxy changed the user's secret.
Each request and multipart upload now holds the backend it uses until
it is finished with it. In-progress multipart uploads are aborted when
the server is shut down so their backends are released.
With --auth-proxy each access key ID may be mapped to a different
backend, but gofakes3 kept a single record of multipart uploads for
everyone. Any user could list the uploads of every other user with
ListMultipartUploads (including their upload IDs) and the parts of
them with ListParts.
With --disable-multipart-streaming the uploads are held by gofakes3,
whose upload IDs are sequential so easy to guess. Any user could
upload parts to, abort, or complete another user's upload - which
wrote it to the backend of the user completing it.
This updates gofakes3 to a version which can keep each user's uploads
private and uses the access key ID as the owner.
With --auth-proxy each access key ID may be mapped to a different
backend, but serve s3 found a streaming multipart upload by its upload
ID alone and stored object metadata by path alone.
A user who learnt another user's upload ID (for example from
ListMultipartUploads on a bucket of the same name) could upload parts
to, complete or abort that upload on the other user's backend. A user
with an object at the same bucket and key as another user was returned
the other user's metadata by HeadObject and GetObject and could
overwrite it.
Keep the uploads and metadata of each access key ID apart.
Nick Craig-Wood