From e9c5340be94044daf3410272af54bc98832dd377 Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Wed, 26 Aug 2026 18:28:06 +0800 Subject: [PATCH] fix: return NoSuchBucket from listing shortcuts ListObjects shortcuts can return EOF before consulting storage, causing missing buckets to appear as empty listings. Verify bucket existence only on those shortcuts so the normal listing path retains the upstream fan-out optimization. Cover ListObjects, ListObjectsV2, and ListObjectVersions at the object layer and verify HTTP 404 NoSuchBucket responses. Fixes #32 Co-authored-by: Jason Lin Co-authored-by: Claude Opus 4.8 (1M context) Signed-off-by: Feng Ruohang --- cmd/bucket-handlers_test.go | 40 ++++++++++++++++++++++++++++++++++++ cmd/metacache-server-pool.go | 16 ++++++++++++--- cmd/object_api_suite_test.go | 39 +++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 3 deletions(-) diff --git a/cmd/bucket-handlers_test.go b/cmd/bucket-handlers_test.go index c8972508f1056..2ae034a503da0 100644 --- a/cmd/bucket-handlers_test.go +++ b/cmd/bucket-handlers_test.go @@ -24,12 +24,52 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "strconv" "testing" "github.com/minio/minio/internal/auth" ) +func TestListObjectsNonExistentBucketHandler(t *testing.T) { + ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{t: t, objAPITest: testListObjectsNonExistentBucketHandler}) +} + +func testListObjectsNonExistentBucketHandler(_ ObjectLayer, instanceType, _ string, apiRouter http.Handler, + credentials auth.Credentials, t *testing.T, +) { + const bucket = "missing-bucket" + testCases := []struct { + name string + query url.Values + }{ + {name: "ListObjects", query: url.Values{"prefix": {"/"}}}, + {name: "ListObjectsV2", query: url.Values{"list-type": {"2"}, "prefix": {"/"}}}, + {name: "ListObjectVersions", query: url.Values{"versions": {""}, "prefix": {"/"}}}, + } + + for _, tc := range testCases { + req, err := newTestSignedRequestV4(http.MethodGet, makeTestTargetURL("", bucket, "", tc.query), 0, nil, + credentials.AccessKey, credentials.SecretKey, nil) + if err != nil { + t.Fatalf("%s: %s: failed to create request: %v", instanceType, tc.name, err) + } + rec := httptest.NewRecorder() + apiRouter.ServeHTTP(rec, req) + if rec.Code != http.StatusNotFound { + t.Errorf("%s: %s: expected status %d, got %d", instanceType, tc.name, http.StatusNotFound, rec.Code) + } + + var apiErr APIErrorResponse + if err = xml.Unmarshal(rec.Body.Bytes(), &apiErr); err != nil { + t.Fatalf("%s: %s: failed to decode error response: %v", instanceType, tc.name, err) + } + if apiErr.Code != "NoSuchBucket" { + t.Errorf("%s: %s: expected NoSuchBucket, got %q", instanceType, tc.name, apiErr.Code) + } + } +} + // Wrapper for calling RemoveBucket HTTP handler tests for both Erasure multiple disks and single node setup. func TestRemoveBucketHandler(t *testing.T) { ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{t: t, objAPITest: testRemoveBucketHandler, endpoints: []string{"RemoveBucket"}}) diff --git a/cmd/metacache-server-pool.go b/cmd/metacache-server-pool.go index bcdfed8f10ff4..c62f822ce4450 100644 --- a/cmd/metacache-server-pool.go +++ b/cmd/metacache-server-pool.go @@ -71,13 +71,13 @@ func (z *erasureServerPools) listPath(ctx context.Context, o *listPathOptions) ( if o.Marker != "" && o.Prefix != "" { // Marker not common with prefix is not implemented. Send an empty response if !HasPrefix(o.Marker, o.Prefix) { - return entries, io.EOF + return entries, z.listPathShortcutEOF(ctx, o.Bucket) } } // With max keys of zero we have reached eof, return right here. if o.Limit == 0 { - return entries, io.EOF + return entries, z.listPathShortcutEOF(ctx, o.Bucket) } // For delimiter and prefix as '/' we do not list anything at all @@ -85,7 +85,7 @@ func (z *erasureServerPools) listPath(ctx context.Context, o *listPathOptions) ( // as '/' we don't have any entries, since all the keys are // of form 'keyName/...' if strings.HasPrefix(o.Prefix, SlashSeparator) { - return entries, io.EOF + return entries, z.listPathShortcutEOF(ctx, o.Bucket) } // If delimiter is slashSeparator we must return directories of @@ -254,6 +254,16 @@ func (z *erasureServerPools) listPath(ctx context.Context, o *listPathOptions) ( return entries, nil } +// listPathShortcutEOF verifies bucket existence for shortcuts that return +// without consulting the storage layer. Keep this check out of the normal +// listing path since GetBucketInfo fans out to peers and disks. +func (z *erasureServerPools) listPathShortcutEOF(ctx context.Context, bucket string) error { + if _, err := z.GetBucketInfo(ctx, bucket, BucketOptions{}); err != nil { + return err + } + return io.EOF +} + // listMerged will list across all sets and return a merged results stream. // The result channel is closed when no more results are expected. func (z *erasureServerPools) listMerged(ctx context.Context, o listPathOptions, results chan<- metaCacheEntry) error { diff --git a/cmd/object_api_suite_test.go b/cmd/object_api_suite_test.go index 3377ad208d2f5..0e037295d4818 100644 --- a/cmd/object_api_suite_test.go +++ b/cmd/object_api_suite_test.go @@ -849,6 +849,45 @@ func testListObjectsTestsForNonExistentBucket(obj ObjectLayer, instanceType stri } } +// Wrapper for calling testListObjectsShortcutsForNonExistentBucket for both +// single-drive and multi-drive erasure setups. +func TestListObjectsShortcutsForNonExistentBucket(t *testing.T) { + ExecObjectLayerTest(t, testListObjectsShortcutsForNonExistentBucket) +} + +// Tests validate that storage-bypassing list shortcuts do not mask a missing +// bucket as an empty result. The regular prefix case is a control for the +// storage-backed listing path. +func testListObjectsShortcutsForNonExistentBucket(obj ObjectLayer, instanceType string, t TestErrHandler) { + testCases := []struct { + name string + prefix string + marker string + maxKeys int + }{ + {name: "slash-prefixed prefix", prefix: "/", maxKeys: 1000}, + {name: "zero limit", prefix: "obj", maxKeys: 0}, + {name: "marker outside prefix", prefix: "a", marker: "b", maxKeys: 1000}, + {name: "regular prefix", prefix: "foo/bar", maxKeys: 1000}, + } + for _, tc := range testCases { + _, err := obj.ListObjects(context.Background(), "bucket", tc.prefix, tc.marker, "", tc.maxKeys) + if !isErrBucketNotFound(err) { + t.Errorf("%s: ListObjects %s: expected BucketNotFound, got %v", instanceType, tc.name, err) + } + + _, err = obj.ListObjectsV2(context.Background(), "bucket", tc.prefix, "", "", tc.maxKeys, false, tc.marker) + if !isErrBucketNotFound(err) { + t.Errorf("%s: ListObjectsV2 %s: expected BucketNotFound, got %v", instanceType, tc.name, err) + } + + _, err = obj.ListObjectVersions(context.Background(), "bucket", tc.prefix, tc.marker, "", "", tc.maxKeys) + if !isErrBucketNotFound(err) { + t.Errorf("%s: ListObjectVersions %s: expected BucketNotFound, got %v", instanceType, tc.name, err) + } + } +} + // Wrapper for calling testNonExistentObjectInBucket for both Erasure and FS. func TestNonExistentObjectInBucket(t *testing.T) { ExecObjectLayerTest(t, testNonExistentObjectInBucket)