From cf4fc3b602798fae998f1232d90dfac07ab1ea58 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Fri, 19 Jan 2024 15:15:15 +0300 Subject: [PATCH] [#165] Extend error on getting listing containers not in current namespace Signed-off-by: Denis Kirillov --- api/handler/object_list_test.go | 109 ++++++++++++++++++++++++++++++++ api/layer/container.go | 2 +- api/layer/listing.go | 10 ++- 3 files changed, 114 insertions(+), 7 deletions(-) diff --git a/api/handler/object_list_test.go b/api/handler/object_list_test.go index 2d5c281..0e9d9b2 100644 --- a/api/handler/object_list_test.go +++ b/api/handler/object_list_test.go @@ -1,6 +1,7 @@ package handler import ( + "fmt" "net/http" "net/url" "sort" @@ -335,6 +336,35 @@ func TestS3BucketListV2DelimiterPercentage(t *testing.T) { require.Equal(t, "c%", listV2Response.CommonPrefixes[1].Prefix) } +func TestS3BucketListDelimiterPrefix(t *testing.T) { + hc := prepareHandlerContext(t) + + bktName := "bucket-for-listing" + bktInfo := createTestBucket(hc, bktName) + + objects := []string{"asdf", "boo/bar", "boo/baz/xyzzy", "cquux/thud", "cquux/bla"} + for _, objName := range objects { + createTestObject(hc, bktInfo, objName, encryption.Params{}) + } + + var empty []string + delim := "/" + prefix := "" + + marker := validateListV1(t, hc, bktName, prefix, delim, "", 1, true, []string{"asdf"}, empty, "asdf") + marker = validateListV1(t, hc, bktName, prefix, delim, marker, 1, true, empty, []string{"boo/"}, "boo/") + validateListV1(t, hc, bktName, prefix, delim, marker, 1, false, empty, []string{"cquux/"}, "") + + marker = validateListV1(t, hc, bktName, prefix, delim, "", 2, true, []string{"asdf"}, []string{"boo/"}, "boo/") + validateListV1(t, hc, bktName, prefix, delim, marker, 2, false, empty, []string{"cquux/"}, "") + + prefix = "boo/" + marker = validateListV1(t, hc, bktName, prefix, delim, "", 1, true, []string{"boo/bar"}, empty, "boo/bar") + validateListV1(t, hc, bktName, prefix, delim, marker, 1, false, empty, []string{"boo/baz/"}, "") + + validateListV1(t, hc, bktName, prefix, delim, "", 2, false, []string{"boo/bar"}, []string{"boo/baz/"}, "") +} + func TestS3BucketListV2DelimiterPrefix(t *testing.T) { tc := prepareHandlerContext(t) @@ -364,6 +394,65 @@ func TestS3BucketListV2DelimiterPrefix(t *testing.T) { validateListV2(t, tc, bktName, prefix, delim, "", 2, false, true, []string{"boo/bar"}, []string{"boo/baz/"}) } +func TestS3BucketListDelimiterPrefixUnderscore(t *testing.T) { + hc := prepareHandlerContext(t) + + bktName := "bucket-for-listing" + bktInfo := createTestBucket(hc, bktName) + + objects := []string{"_obj1_", "_under1/bar", "_under1/baz/xyzzy", "_under2/thud", "_under2/bla"} + for _, objName := range objects { + createTestObject(hc, bktInfo, objName, encryption.Params{}) + } + + var empty []string + delim := "/" + prefix := "" + + marker := validateListV1(t, hc, bktName, prefix, delim, "", 1, true, []string{"_obj1_"}, empty, "_obj1_") + marker = validateListV1(t, hc, bktName, prefix, delim, marker, 1, true, empty, []string{"_under1/"}, "_under1/") + validateListV1(t, hc, bktName, prefix, delim, marker, 1, false, empty, []string{"_under2/"}, "") + + marker = validateListV1(t, hc, bktName, prefix, delim, "", 2, true, []string{"_obj1_"}, []string{"_under1/"}, "_under1/") + validateListV1(t, hc, bktName, prefix, delim, marker, 2, false, empty, []string{"_under2/"}, "") + + prefix = "_under1/" + marker = validateListV1(t, hc, bktName, prefix, delim, "", 1, true, []string{"_under1/bar"}, empty, "_under1/bar") + validateListV1(t, hc, bktName, prefix, delim, marker, 1, false, empty, []string{"_under1/baz/"}, "") + + validateListV1(t, hc, bktName, prefix, delim, "", 2, false, []string{"_under1/bar"}, []string{"_under1/baz/"}, "") +} + +func TestS3BucketListDelimiterNotSkipSpecial(t *testing.T) { + hc := prepareHandlerContext(t) + + bktName := "bucket-for-listing" + bktInfo := createTestBucket(hc, bktName) + + objects := []string{"0/"} + for i := 1000; i < 1999; i++ { + objects = append(objects, fmt.Sprintf("0/%d", i)) + } + + objects2 := []string{"1999", "1999#", "1999+", "2000"} + objects = append(objects, objects2...) + + for _, objName := range objects { + createTestObject(hc, bktInfo, objName, encryption.Params{}) + } + + delimiter := "/" + list := listObjectsV1(hc, bktName, "", delimiter, "", -1) + + require.Equal(t, delimiter, list.Delimiter) + require.Equal(t, []CommonPrefix{{Prefix: "0/"}}, list.CommonPrefixes) + + require.Len(t, list.Contents, len(objects2)) + for i := 0; i < len(list.Contents); i++ { + require.Equal(t, objects2[i], list.Contents[i].Key) + } +} + func TestMintVersioningListObjectVersionsVersionIDContinuation(t *testing.T) { hc := prepareHandlerContext(t) @@ -482,6 +571,26 @@ func listObjectsV2Ext(hc *handlerContext, bktName, prefix, delimiter, startAfter return res } +func validateListV1(t *testing.T, tc *handlerContext, bktName, prefix, delimiter, marker string, maxKeys int, + isTruncated bool, checkObjects, checkPrefixes []string, nextMarker string) string { + response := listObjectsV1(tc, bktName, prefix, delimiter, marker, maxKeys) + + require.Equal(t, isTruncated, response.IsTruncated) + require.Equal(t, nextMarker, response.NextMarker) + + require.Len(t, response.Contents, len(checkObjects)) + for i := 0; i < len(checkObjects); i++ { + require.Equal(t, checkObjects[i], response.Contents[i].Key) + } + + require.Len(t, response.CommonPrefixes, len(checkPrefixes)) + for i := 0; i < len(checkPrefixes); i++ { + require.Equal(t, checkPrefixes[i], response.CommonPrefixes[i].Prefix) + } + + return response.NextMarker +} + func validateListV2(t *testing.T, tc *handlerContext, bktName, prefix, delimiter, continuationToken string, maxKeys int, isTruncated, last bool, checkObjects, checkPrefixes []string) string { response := listObjectsV2(tc, bktName, prefix, delimiter, "", continuationToken, maxKeys) diff --git a/api/layer/container.go b/api/layer/container.go index e5085a3..e3cba2d 100644 --- a/api/layer/container.go +++ b/api/layer/container.go @@ -76,7 +76,7 @@ func (n *layer) containerInfo(ctx context.Context, idCnr cid.ID) (*data.BucketIn zone, _ := n.features.FormContainerZone(reqInfo.Namespace) if zone != info.Zone { - return nil, fmt.Errorf("ns '%s' and zone '%s' are mismatched", zone, info.Zone) + return nil, fmt.Errorf("ns '%s' and zone '%s' are mismatched for container '%s'", zone, info.Zone, idCnr) } n.cache.PutBucket(info) diff --git a/api/layer/listing.go b/api/layer/listing.go index 8e45683..ba2b6f3 100644 --- a/api/layer/listing.go +++ b/api/layer/listing.go @@ -296,11 +296,6 @@ func (n *layer) getLatestObjectsVersions(ctx context.Context, p allObjectParams) return nil, nil, fmt.Errorf("failed to get next object from stream: %w", err) } - // probably isn't necessary - sort.Slice(objects, func(i, j int) bool { - return objects[i].FilePath < objects[j].FilePath - }) - if len(objects) > p.MaxKeys { next = objects[p.MaxKeys] objects = objects[:p.MaxKeys] @@ -523,7 +518,10 @@ func (n *layer) initWorkerPoolStream(ctx context.Context, size int, p allObjectP default: } - if node.IsFilledExtra() || tryDirectoryName(node, p.Prefix, p.Delimiter) != "" { // todo think to not compute twice + if dirName := tryDirectoryName(node, p.Prefix, p.Delimiter); dirName != "" || node.IsFilledExtra() { // todo think to not compute twice + if dirName != "" { + node.FilePath = dirName + } select { case <-ctx.Done(): case objCh <- node: