fix(server/s3): paginate recursive object listings (#2968)

fix(s3): paginate recursive object listings

- stop recursive traversal after filling the requested S3 page
- preserve lexicographic marker ordering and request cancellation
- cover bounded traversal, continuation, prefixes, and cache safety

Co-authored-by: nostalume <nostalucent@gmail.com>
Co-authored-by: Codex <267193182+codex@users.noreply.github.com>

Author: 3 people

server/s3/backend.go

@@ -36,13 +36,15 @@ var (
 // s3Backend implements the gofacess3.Backend interface to make an S3
 // backend for gofakes3
 type s3Backend struct {
-	meta *sync.Map
+	meta    *sync.Map
+	listDir func(context.Context, string) ([]model.Obj, error)
 }
 
 // newBackend creates a new SimpleBucketBackend.
 func newBackend() gofakes3.Backend {
 	return &s3Backend{
-		meta: new(sync.Map),
+		meta:    new(sync.Map),
+		listDir: getDirEntries,
 	}
 }
 
@@ -84,18 +86,17 @@ func (b *s3Backend) ListBucket(ctx context.Context, bucketName string, prefix *g
 		prefix.HasDelimiter = false
 	}
 
-	response := gofakes3.NewObjectList()
 	path, remaining := prefixParser(prefix)
 
-	err = b.entryListR(bucketPath, path, remaining, prefix.HasDelimiter, response)
+	response, err := b.listPage(ctx, bucketPath, path, remaining, prefix.HasDelimiter, page)
 	if err == gofakes3.ErrNoSuchKey {
 		// AWS just returns an empty list
 		response = gofakes3.NewObjectList()
 	} else if err != nil {
 		return nil, err
 	}
 
-	return b.pager(response, page)
+	return response, nil
 }
 
 // HeadObject returns the fileinfo for the given object name.

server/s3/list.go

@@ -3,24 +3,118 @@
 package s3
 
 import (
+	"context"
 	"path"
+	"slices"
+	"sort"
 	"strings"
 	"time"
 
 	"github.com/itsHenry35/gofakes3"
 	log "github.com/sirupsen/logrus"
 )
 
-func (b *s3Backend) entryListR(bucket, fdPath, name string, addPrefix bool, response *gofakes3.ObjectList) error {
-	fp := path.Join(bucket, fdPath)
+// S3 ListObjects responses contain at most 1,000 keys.
+const maxListPageKeys int64 = 1000
+
+type objectPage struct {
+	result  *gofakes3.ObjectList
+	marker  string
+	maxKeys int64
+	count   int64
+	lastKey string
+}
+
+func newObjectPage(page gofakes3.ListBucketPage) *objectPage {
+	maxKeys := page.MaxKeys
+	if maxKeys <= 0 || maxKeys > maxListPageKeys {
+		maxKeys = maxListPageKeys
+	}
+	marker := ""
+	if page.HasMarker {
+		marker = page.Marker
+	}
+	return &objectPage{
+		result:  gofakes3.NewObjectList(),
+		marker:  marker,
+		maxKeys: maxKeys,
+	}
+}
+
+func (p *objectPage) addContent(item *gofakes3.Content) bool {
+	if item.Key <= p.marker {
+		return true
+	}
+	if p.count >= p.maxKeys {
+		p.result.IsTruncated = true
+		return false
+	}
+	p.result.Add(item)
+	p.count++
+	p.lastKey = item.Key
+	return true
+}
+
+func (p *objectPage) addPrefix(prefix string) bool {
+	if prefix <= p.marker {
+		return true
+	}
+	if p.count >= p.maxKeys {
+		p.result.IsTruncated = true
+		return false
+	}
+	p.result.AddPrefix(prefix)
+	p.count++
+	p.lastKey = prefix
+	return true
+}
 
-	dirEntries, err := getDirEntries(fp)
+func (p *objectPage) finish() *gofakes3.ObjectList {
+	if p.result.IsTruncated {
+		p.result.NextMarker = p.lastKey
+	}
+	return p.result
+}
+
+func (b *s3Backend) listPage(
+	ctx context.Context,
+	bucket, fdPath, name string,
+	addPrefix bool,
+	page gofakes3.ListBucketPage,
+) (*gofakes3.ObjectList, error) {
+	result := newObjectPage(page)
+	_, err := b.walkPage(ctx, bucket, fdPath, name, addPrefix, result)
+	if err != nil {
+		return nil, err
+	}
+	return result.finish(), nil
+}
+
+// walkPage returns false after finding an entry beyond the requested page.
+func (b *s3Backend) walkPage(
+	ctx context.Context,
+	bucket, fdPath, name string,
+	addPrefix bool,
+	page *objectPage,
+) (bool, error) {
+	if err := ctx.Err(); err != nil {
+		return false, err
+	}
+
+	fp := path.Join(bucket, fdPath)
+	dirEntries, err := b.listDir(ctx, fp)
 	if err != nil {
-		return err
+		return false, err
+	}
+	if err := ctx.Err(); err != nil {
+		return false, err
 	}
 
 	// workaround as s3 can't have empty files in directories, useful in deletions
 	if len(dirEntries) == 0 {
+		if !strings.HasPrefix(emptyObjectName, name) {
+			return true, nil
+		}
 		item := &gofakes3.Content{
 			// Key:          gofakes3.URLEncode(path.Join(fdPath, emptyObjectName)),
 			Key:          path.Join(fdPath, emptyObjectName),
@@ -29,11 +123,15 @@ func (b *s3Backend) entryListR(bucket, fdPath, name string, addPrefix bool, resp
 			Size:         0,
 			StorageClass: gofakes3.StorageStandard,
 		}
-		response.Add(item)
 		log.Debugf("Adding empty object %s to response", item.Key)
-		return nil
+		return page.addContent(item), nil
 	}
 
+	dirEntries = slices.Clone(dirEntries)
+	sort.Slice(dirEntries, func(i, j int) bool {
+		return dirEntries[i].GetName() < dirEntries[j].GetName()
+	})
+
 	for _, entry := range dirEntries {
 		object := entry.GetName()
 
@@ -47,12 +145,19 @@ func (b *s3Backend) entryListR(bucket, fdPath, name string, addPrefix bool, resp
 		if entry.IsDir() {
 			if addPrefix {
 				// response.AddPrefix(gofakes3.URLEncode(objectPath))
-				response.AddPrefix(objectPath)
+				if !page.addPrefix(objectPath) {
+					return false, nil
+				}
+				continue
+			}
+			subtreePrefix := objectPath + "/"
+			// A marker beyond this subtree lets us avoid an upstream directory read.
+			if subtreePrefix <= page.marker && !strings.HasPrefix(page.marker, subtreePrefix) {
 				continue
 			}
-			err := b.entryListR(bucket, path.Join(fdPath, object), "", false, response)
-			if err != nil {
-				return err
+			keepGoing, err := b.walkPage(ctx, bucket, objectPath, "", false, page)
+			if err != nil || !keepGoing {
+				return keepGoing, err
 			}
 		} else {
 			item := &gofakes3.Content{
@@ -63,8 +168,10 @@ func (b *s3Backend) entryListR(bucket, fdPath, name string, addPrefix bool, resp
 				Size:         entry.GetSize(),
 				StorageClass: gofakes3.StorageStandard,
 			}
-			response.Add(item)
+			if !page.addContent(item) {
+				return false, nil
+			}
 		}
 	}
-	return nil
+	return true, nil
 }