From dfb0b240f9f6dda362c8aa2e4a302077a57343c2 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 19 Feb 2025 20:30:52 -0800 Subject: [PATCH] tags were incorrectly used, fix them use the right package (#2070) --- api-compose-object.go | 9 +++-- api-put-object.go | 5 ++- pkg/s3utils/utils.go | 38 -------------------- pkg/s3utils/utils_test.go | 76 --------------------------------------- utils.go | 11 ++++-- 5 files changed, 19 insertions(+), 120 deletions(-) diff --git a/api-compose-object.go b/api-compose-object.go index bb595626e..2574c135a 100644 --- a/api-compose-object.go +++ b/api-compose-object.go @@ -30,6 +30,7 @@ import ( "github.com/google/uuid" "github.com/minio/minio-go/v7/pkg/encrypt" "github.com/minio/minio-go/v7/pkg/s3utils" + "github.com/minio/minio-go/v7/pkg/tags" ) // CopyDestOptions represents options specified by user for CopyObject/ComposeObject APIs @@ -98,8 +99,8 @@ func (opts CopyDestOptions) Marshal(header http.Header) { const replaceDirective = "REPLACE" if opts.ReplaceTags { header.Set(amzTaggingHeaderDirective, replaceDirective) - if tags := s3utils.TagEncode(opts.UserTags); tags != "" { - header.Set(amzTaggingHeader, tags) + if tags, _ := tags.NewTags(opts.UserTags, true); tags != nil { + header.Set(amzTaggingHeader, tags.String()) } } @@ -236,7 +237,9 @@ func (c *Client) copyObjectDo(ctx context.Context, srcBucket, srcObject, destBuc } if len(dstOpts.UserTags) != 0 { - headers.Set(amzTaggingHeader, s3utils.TagEncode(dstOpts.UserTags)) + if tags, _ := tags.NewTags(dstOpts.UserTags, true); tags != nil { + headers.Set(amzTaggingHeader, tags.String()) + } } reqMetadata := requestMetadata{ diff --git a/api-put-object.go b/api-put-object.go index 098175784..ce4834790 100644 --- a/api-put-object.go +++ b/api-put-object.go @@ -30,6 +30,7 @@ import ( "github.com/minio/minio-go/v7/pkg/encrypt" "github.com/minio/minio-go/v7/pkg/s3utils" + "github.com/minio/minio-go/v7/pkg/tags" "golang.org/x/net/http/httpguts" ) @@ -229,7 +230,9 @@ func (opts PutObjectOptions) Header() (header http.Header) { } if len(opts.UserTags) != 0 { - header.Set(amzTaggingHeader, s3utils.TagEncode(opts.UserTags)) + if tags, _ := tags.NewTags(opts.UserTags, true); tags != nil { + header.Set(amzTaggingHeader, tags.String()) + } } for k, v := range opts.UserMetadata { diff --git a/pkg/s3utils/utils.go b/pkg/s3utils/utils.go index 0fa804dc1..eb631249b 100644 --- a/pkg/s3utils/utils.go +++ b/pkg/s3utils/utils.go @@ -261,44 +261,6 @@ func QueryEncode(v url.Values) string { return buf.String() } -// TagDecode - decodes canonical tag into map of key and value. -func TagDecode(ctag string) map[string]string { - if ctag == "" { - return map[string]string{} - } - tags := strings.Split(ctag, "&") - tagMap := make(map[string]string, len(tags)) - var err error - for _, tag := range tags { - kvs := strings.SplitN(tag, "=", 2) - if len(kvs) == 0 { - return map[string]string{} - } - if len(kvs) == 1 { - return map[string]string{} - } - tagMap[kvs[0]], err = url.PathUnescape(kvs[1]) - if err != nil { - continue - } - } - return tagMap -} - -// TagEncode - encodes tag values in their URL encoded form. In -// addition to the percent encoding performed by urlEncodePath() used -// here, it also percent encodes '/' (forward slash) -func TagEncode(tags map[string]string) string { - if tags == nil { - return "" - } - values := url.Values{} - for k, v := range tags { - values[k] = []string{v} - } - return QueryEncode(values) -} - // if object matches reserved string, no need to encode them var reservedObjectNames = regexp.MustCompile("^[a-zA-Z0-9-_.~/]+$") diff --git a/pkg/s3utils/utils_test.go b/pkg/s3utils/utils_test.go index 86db044cf..f9aceaaf2 100644 --- a/pkg/s3utils/utils_test.go +++ b/pkg/s3utils/utils_test.go @@ -20,7 +20,6 @@ package s3utils import ( "errors" "net/url" - "reflect" "testing" ) @@ -298,81 +297,6 @@ func TestQueryEncode(t *testing.T) { } } -// Tests tag decode to map -func TestTagDecode(t *testing.T) { - testCases := []struct { - // canonical input - canonicalInput string - - // Expected result. - resultMap map[string]string - }{ - {"k=thisisthe%25url", map[string]string{"k": "thisisthe%url"}}, - {"k=%E6%9C%AC%E8%AA%9E", map[string]string{"k": "本語"}}, - {"k=%E6%9C%AC%E8%AA%9E.1", map[string]string{"k": "本語.1"}}, - {"k=%3E123", map[string]string{"k": ">123"}}, - {"k=myurl%23link", map[string]string{"k": "myurl#link"}}, - {"k=space%20in%20url", map[string]string{"k": "space in url"}}, - {"k=url%2Bpath", map[string]string{"k": "url+path"}}, - {"k=url%2Fpath", map[string]string{"k": "url/path"}}, - } - - for _, testCase := range testCases { - testCase := testCase - t.Run("", func(t *testing.T) { - gotResult := TagDecode(testCase.canonicalInput) - if !reflect.DeepEqual(testCase.resultMap, gotResult) { - t.Errorf("Expected %s, got %s", testCase.resultMap, gotResult) - } - }) - } -} - -// Tests tag encode function for user tags. -func TestTagEncode(t *testing.T) { - testCases := []struct { - // Input. - inputMap map[string]string - // Expected result. - result string - }{ - {map[string]string{ - "k": "thisisthe%url", - }, "k=thisisthe%25url"}, - {map[string]string{ - "k": "本語", - }, "k=%E6%9C%AC%E8%AA%9E"}, - {map[string]string{ - "k": "本語.1", - }, "k=%E6%9C%AC%E8%AA%9E.1"}, - {map[string]string{ - "k": ">123", - }, "k=%3E123"}, - {map[string]string{ - "k": "myurl#link", - }, "k=myurl%23link"}, - {map[string]string{ - "k": "space in url", - }, "k=space%20in%20url"}, - {map[string]string{ - "k": "url+path", - }, "k=url%2Bpath"}, - {map[string]string{ - "k": "url/path", - }, "k=url%2Fpath"}, - } - - for _, testCase := range testCases { - testCase := testCase - t.Run("", func(t *testing.T) { - gotResult := TagEncode(testCase.inputMap) - if testCase.result != gotResult { - t.Errorf("Expected %s, got %s", testCase.result, gotResult) - } - }) - } -} - // Tests validate the URL path encoder. func TestEncodePath(t *testing.T) { testCases := []struct { diff --git a/utils.go b/utils.go index cd7d2c27e..027bb6ce3 100644 --- a/utils.go +++ b/utils.go @@ -41,6 +41,7 @@ import ( md5simd "github.com/minio/md5-simd" "github.com/minio/minio-go/v7/pkg/s3utils" + "github.com/minio/minio-go/v7/pkg/tags" ) func trimEtag(etag string) string { @@ -322,7 +323,13 @@ func ToObjectInfo(bucketName, objectName string, h http.Header) (ObjectInfo, err userMetadata[strings.TrimPrefix(k, "X-Amz-Meta-")] = v[0] } } - userTags := s3utils.TagDecode(h.Get(amzTaggingHeader)) + + userTags, err := tags.ParseObjectTags(h.Get(amzTaggingHeader)) + if err != nil { + return ObjectInfo{}, ErrorResponse{ + Code: "InternalError", + } + } var tagCount int if count := h.Get(amzTaggingCount); count != "" { @@ -373,7 +380,7 @@ func ToObjectInfo(bucketName, objectName string, h http.Header) (ObjectInfo, err // which are not part of object metadata. Metadata: metadata, UserMetadata: userMetadata, - UserTags: userTags, + UserTags: userTags.ToMap(), UserTagCount: tagCount, Restore: restore,