From b80e7e8a7e58bc735d563317fc0eff6b09079fc4 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Thu, 15 Oct 2026 13:14:26 +0100 Subject: [PATCH] serve s3: support listing with delimiters other than "/" 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. --- cmd/serve/s3/backend.go | 15 ++---- cmd/serve/s3/list.go | 110 ++++++++++++++++++++++++++++++++++---- cmd/serve/s3/list_test.go | 69 ++++++++++++++++++++++++ cmd/serve/s3/serve_s3.md | 11 ++-- cmd/serve/s3/utils.go | 8 --- 5 files changed, 182 insertions(+), 31 deletions(-) diff --git a/cmd/serve/s3/backend.go b/cmd/serve/s3/backend.go index f29f4f575..8da2e5830 100644 --- a/cmd/serve/s3/backend.go +++ b/cmd/serve/s3/backend.go @@ -272,18 +272,13 @@ func (b *s3Backend) ListBucket(ctx context.Context, bucket string, prefix *gofak prefix = emptyPrefix } - // workaround - if strings.TrimSpace(prefix.Prefix) == "" { - prefix.HasPrefix = false - } - if strings.TrimSpace(prefix.Delimiter) == "" { - prefix.HasDelimiter = false - } - response := gofakes3.NewObjectList() - path, remaining := prefixParser(prefix) + // Keys can't start with "/" so such a prefix matches nothing + if strings.HasPrefix(prefix.Prefix, "/") { + return response, nil + } - err = b.listPage(_vfs, bucket, path, remaining, prefix.HasDelimiter, page, response) + err = b.listPage(_vfs, bucket, prefix.Prefix, prefix.Delimiter, page, response) if err == gofakes3.ErrNoSuchKey { // AWS just returns an empty list response = gofakes3.NewObjectList() diff --git a/cmd/serve/s3/list.go b/cmd/serve/s3/list.go index eb71d5021..f174d0037 100644 --- a/cmd/serve/s3/list.go +++ b/cmd/serve/s3/list.go @@ -38,6 +38,13 @@ type lister struct { keys int // keys added to the page so far lastKey string // the last key added to the page dirsRead int // directories read, for debug logging + + // With a delimiter other than "/", which can't be found by + // listing directories, the whole tree is walked and the keys are + // rolled up into common prefixes. + prefix string // the prefix of the listing + delimiter string // the delimiter, or "" if none or "/" + lastPrefix string // the last common prefix added to the page } // newLister makes a lister which fills response with at most one page of the @@ -128,6 +135,65 @@ func (l *lister) addObject(key string, entry vfs.Node) error { return nil } +// commonPrefix returns the common prefix key rolls up into, if the +// listing has a delimiter other than "/" and key has it after the +// listing's prefix. +func (l *lister) commonPrefix(key string) (prefix string, ok bool) { + if l.delimiter == "" || !strings.HasPrefix(key, l.prefix) { + return "", false + } + i := strings.Index(key[len(l.prefix):], l.delimiter) + if i < 0 { + return "", false + } + return key[:len(l.prefix)+i+len(l.delimiter)], true +} + +// addRolledUp adds the common prefix a key rolls up into to the page +// unless it has been added already, returning errPageFull if the page +// is now complete. +// +// The keys rolling up into a common prefix are contiguous in the +// listing, so it only needs comparing with the last one. +func (l *lister) addRolledUp(prefix string) error { + if prefix == l.lastPrefix || l.skipKey(prefix) { + return nil + } + if err := l.addCommonPrefix(prefix); err != nil { + return err + } + l.lastPrefix = prefix + return nil +} + +// hasKeys reports whether the directory fp in the bucket has any keys +// below it. +func (l *lister) hasKeys(fdPath string) bool { + fp, err := bucketDirPath(l.bucket, fdPath) + if err != nil { + return false + } + dirEntries, err := getDirEntries(fp, l.vfs) + if err != nil { + return false + } + for _, entry := range dirEntries { + if isTempObject(entry.Name()) { + continue + } + if !entry.IsDir() || l.hasKeys(path.Join(fdPath, entry.Name())) { + return true + } + } + return false +} + +// isTempObject reports whether name is a temporary object of an +// in-progress upload, which is hidden from listings. +func isTempObject(name string) bool { + return strings.HasPrefix(name, tempObjectPrefix) || strings.HasPrefix(name, legacyMultipartUploadPrefix) +} + // list walks the directory fdPath, adding the entries whose leaf name starts // with name to the page. // @@ -159,7 +225,7 @@ func (l *lister) list(fdPath, name string, addPrefix bool) error { object := entry.Name() // Hide the temporary objects of in-progress uploads - if strings.HasPrefix(object, tempObjectPrefix) || strings.HasPrefix(object, legacyMultipartUploadPrefix) { + if isTempObject(object) { continue } @@ -184,6 +250,17 @@ func (l *lister) list(fdPath, name string, addPrefix bool) error { if l.skipDir(prefixWithTrailingSlash) { continue } + // If the delimiter is in the directory's path every + // key below it rolls up into the same common prefix, + // so the directory needn't be read. + if prefix, ok := l.commonPrefix(prefixWithTrailingSlash); ok { + if l.hasKeys(objectPath) { + if err := l.addRolledUp(prefix); err != nil { + return err + } + } + continue + } err := l.list(objectPath, "", false) if errors.Is(err, gofakes3.ErrNoSuchKey) { // The directory went away while we were listing it, so @@ -193,6 +270,12 @@ func (l *lister) list(fdPath, name string, addPrefix bool) error { return err } } else { + if prefix, ok := l.commonPrefix(objectPath); ok { + if err := l.addRolledUp(prefix); err != nil { + return err + } + continue + } if l.skipKey(objectPath) { continue } @@ -204,16 +287,25 @@ func (l *lister) list(fdPath, name string, addPrefix bool) error { return nil } -// listPage fills response with one page of the listing of the directory -// fdPath in bucket, as described by page. +// listPage fills response with one page of the listing of bucket with +// prefix and delimiter, as described by page. // -// Only entries whose leaf name starts with name are listed. If addPrefix is -// set, subdirectories are reported as common prefixes, otherwise the whole -// subtree below fdPath is listed. -// -// It returns gofakes3.ErrNoSuchKey if fdPath is not a directory. -func (b *s3Backend) listPage(_vfs *vfs.VFS, bucket, fdPath, name string, addPrefix bool, page gofakes3.ListBucketPage, response *gofakes3.ObjectList) error { +// It returns gofakes3.ErrNoSuchKey if the directory the prefix is in +// doesn't exist. +func (b *s3Backend) listPage(_vfs *vfs.VFS, bucket, prefix, delimiter string, page gofakes3.ListBucketPage, response *gofakes3.ObjectList) error { l := newLister(b, _vfs, bucket, page, response) + // The prefix is split into the directory to list and the start of + // the names in it to list. + fdPath, name := "", prefix + if i := strings.LastIndexByte(prefix, '/'); i >= 0 { + fdPath, name = prefix[:i], prefix[i+1:] + } + // A "/" delimiter is found by listing directories, any other by + // walking the whole tree. + addPrefix := delimiter == "/" + if delimiter != "" && delimiter != "/" { + l.prefix, l.delimiter = prefix, delimiter + } err := l.list(fdPath, name, addPrefix) if err != nil && !errors.Is(err, errPageFull) { return err diff --git a/cmd/serve/s3/list_test.go b/cmd/serve/s3/list_test.go index 575f3313c..0851162a1 100644 --- a/cmd/serve/s3/list_test.go +++ b/cmd/serve/s3/list_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "slices" + "strings" "testing" "github.com/rclone/gofakes3" @@ -263,6 +264,74 @@ func TestListSkipDir(t *testing.T) { } } +// TestListAnyDelimiter checks listings with any delimiter, and with a +// prefix, against the keys an S3 listing of the same flat keyspace has, +// whatever the page size. +func TestListAnyDelimiter(t *testing.T) { + files := []string{ + "a.txt", "a/b/c", "a/bc", "a0", + "b%ar", "b ar", "b.ar", "bar", "bazar", "baz/qux", "baz/q ux/x", + "cab", "c/a/b/z", "c/ab", "dir/sub/file", "foo", + } + var paths []string + for _, file := range files { + paths = append(paths, "bucket/"+file) + } + b := newListBackend(t, paths...) + // An empty directory has no keys, so mustn't be a common prefix + // for delimiters other than "/" + require.NoError(t, os.MkdirAll(filepath.Join(b.s.f.Root(), "bucket", "empty", "abc"), 0777)) + + // want returns the keys and common prefixes S3 lists + want := func(prefix, delimiter string) (keys []string) { + seen := map[string]bool{} + keySpace := files + if delimiter == "/" { + // Listing directories with "/" shows empty ones too, as + // it always has + keySpace = append(slices.Clone(files), "empty/abc/") + } + for _, key := range keySpace { + if !strings.HasPrefix(key, prefix) { + continue + } + if delimiter != "" { + if i := strings.Index(key[len(prefix):], delimiter); i >= 0 { + key = key[:len(prefix)+i+len(delimiter)] + } + } + if !seen[key] { + seen[key] = true + keys = append(keys, key) + } + } + slices.Sort(keys) + return keys + } + + for _, prefix := range []string{"", "b", "ba", "baz/", "c", "c/a", "e", "x"} { + for _, delimiter := range []string{"", "/", "a", "ba", " ", ".", "%", "/a", "b/", "z"} { + t.Run(fmt.Sprintf("prefix=%q,delimiter=%q", prefix, delimiter), func(t *testing.T) { + expected := want(prefix, delimiter) + for _, maxKeys := range []int{1, 2, 3, 1000} { + got := listPages(t, b, listPrefix(prefix, delimiter), maxKeys) + assert.Equal(t, expected, got, "max-keys %d", maxKeys) + } + }) + } + } +} + +// TestListPrefixLeadingSlash checks a prefix starting with "/" matches +// nothing, as no key starts with "/". +func TestListPrefixLeadingSlash(t *testing.T) { + b := newListBackend(t, "bucket/a/b", "bucket/c") + keys, _ := listOnce(t, b, listPrefix("/", ""), gofakes3.ListBucketPage{MaxKeys: 1000}) + assert.Empty(t, keys) + keys, _ = listOnce(t, b, listPrefix("/a", "/"), gofakes3.ListBucketPage{MaxKeys: 1000}) + assert.Empty(t, keys) +} + // TestListETagQuoted checks the ETags in listings are quoted as on S3 // and in HEAD responses. func TestListETagQuoted(t *testing.T) { diff --git a/cmd/serve/s3/serve_s3.md b/cmd/serve/s3/serve_s3.md index 529b7d37f..fcc91a1cc 100644 --- a/cmd/serve/s3/serve_s3.md +++ b/cmd/serve/s3/serve_s3.md @@ -441,10 +441,13 @@ When using `PutObject` or `DeleteObject`, rclone will automatically create or clean up empty folders. If you don't want to clean up empty folders automatically, use `--no-cleanup`. -When using `ListObjects`, rclone treats any delimiter as `/`. Without a -delimiter the listing is recursive: rclone walks the directory tree -below the prefix, reading only as many directories as it needs to fill -each page. +When using `ListObjects` with the delimiter `/`, rclone lists the +directory the prefix is in, showing its subdirectories as common +prefixes, including empty ones. Without a delimiter the listing is +recursive: rclone walks the directory tree below the prefix, reading +only as many directories as it needs to fill each page. Any other +delimiter is supported too, but needs the whole directory tree below +the prefix walking, unless the delimiter is in the directory names. Versioning is not currently supported. diff --git a/cmd/serve/s3/utils.go b/cmd/serve/s3/utils.go index e5f21a14c..e6498874d 100644 --- a/cmd/serve/s3/utils.go +++ b/cmd/serve/s3/utils.go @@ -130,14 +130,6 @@ func bucketDirPath(bucketName, dirName string) (string, error) { return bucketObjectPath(bucketName, dirName) } -func prefixParser(p *gofakes3.Prefix) (path, remaining string) { - idx := strings.LastIndexByte(p.Prefix, '/') - if idx < 0 { - return "", p.Prefix - } - return p.Prefix[:idx], p.Prefix[idx+1:] -} - // mkdirRecursive makes the directory path and any of its parents which // don't exist, returning the top-most directory it made, or "" if path // existed already.