mirror of
https://github.com/pgsty/minio.git
synced 2026-09-05 18:16:16 +03:00
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 <jason@JasondeMacBook-Air.local> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
@@ -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"}})
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user