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)