From b0e2c2da781f8b9568a0532672586114b5e59687 Mon Sep 17 00:00:00 2001
From: Anis Elleuch <vadmeste@users.noreply.github.com>
Date: Sat, 14 May 2022 18:25:55 +0100
Subject: [PATCH] lifecycle: Support tags with special characters (#14906)

Object tags can have special characters such as whitespace. However
the current code doesn't properly consider those characters while
evaluating the lifecycle document.

ObjectInfo.UserTags contains an url encoded form of object tags
(e.g. key+1=val)

This commit fixes the issue by using the tags package to parse object tags.
---
 internal/bucket/lifecycle/filter.go         | 51 ++++++++++++++-------
 internal/bucket/lifecycle/lifecycle.go      |  2 +-
 internal/bucket/lifecycle/lifecycle_test.go |  8 ++++
 3 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/internal/bucket/lifecycle/filter.go b/internal/bucket/lifecycle/filter.go
index f881e99b1..aa8ef455b 100644
--- a/internal/bucket/lifecycle/filter.go
+++ b/internal/bucket/lifecycle/filter.go
@@ -20,6 +20,9 @@ package lifecycle
 import (
 	"encoding/xml"
 	"io"
+	"log"
+
+	"github.com/minio/minio-go/v7/pkg/tags"
 )
 
 var errInvalidFilter = Errorf("Filter must have exactly one of Prefix, Tag, or And specified")
@@ -36,8 +39,9 @@ type Filter struct {
 
 	Tag    Tag
 	tagSet bool
+
 	// Caching tags, only once
-	cachedTags []string
+	cachedTags map[string]string
 }
 
 // MarshalXML - produces the xml representation of the Filter struct
@@ -150,27 +154,42 @@ func (f Filter) Validate() error {
 
 // TestTags tests if the object tags satisfy the Filter tags requirement,
 // it returns true if there is no tags in the underlying Filter.
-func (f Filter) TestTags(tags []string) bool {
+func (f Filter) TestTags(userTags string) bool {
 	if f.cachedTags == nil {
-		tags := make([]string, 0)
+		cache := make(map[string]string)
 		for _, t := range append(f.And.Tags, f.Tag) {
 			if !t.IsEmpty() {
-				tags = append(tags, t.String())
+				cache[t.Key] = t.Value
 			}
 		}
-		f.cachedTags = tags
+		f.cachedTags = cache
 	}
-	for _, ct := range f.cachedTags {
-		foundTag := false
-		for _, t := range tags {
-			if ct == t {
-				foundTag = true
-				break
-			}
-		}
-		if !foundTag {
-			return false
+
+	// This filter does not have any tags, always return true
+	if len(f.cachedTags) == 0 {
+		return true
+	}
+
+	parsedTags, err := tags.ParseObjectTags(userTags)
+	if err != nil {
+		log.Printf("Unexpected object tag found: `%s`\n", userTags)
+		return false
+	}
+	tagsMap := parsedTags.ToMap()
+
+	// This filter has tags configured but this object
+	// does not have any tag, skip this object
+	if len(tagsMap) == 0 {
+		return false
+	}
+
+	// Both filter and object have tags, find a match,
+	// skip this object otherwise
+	for k, cv := range f.cachedTags {
+		v, ok := tagsMap[k]
+		if ok && v == cv {
+			return true
 		}
 	}
-	return true
+	return false
 }
diff --git a/internal/bucket/lifecycle/lifecycle.go b/internal/bucket/lifecycle/lifecycle.go
index 594726415..9e70c2ea1 100644
--- a/internal/bucket/lifecycle/lifecycle.go
+++ b/internal/bucket/lifecycle/lifecycle.go
@@ -259,7 +259,7 @@ func (lc Lifecycle) FilterActionableRules(obj ObjectOpts) []Rule {
 			continue
 		}
 
-		if rule.Filter.TestTags(strings.Split(obj.UserTags, "&")) {
+		if rule.Filter.TestTags(obj.UserTags) {
 			rules = append(rules, rule)
 		}
 		if !rule.Transition.IsNull() {
diff --git a/internal/bucket/lifecycle/lifecycle_test.go b/internal/bucket/lifecycle/lifecycle_test.go
index f5fd3d8a7..14b0166e9 100644
--- a/internal/bucket/lifecycle/lifecycle_test.go
+++ b/internal/bucket/lifecycle/lifecycle_test.go
@@ -325,6 +325,14 @@ func TestComputeActions(t *testing.T) {
 			objectModTime:  time.Now().UTC().Add(-24 * time.Hour), // Created 1 day ago
 			expectedAction: DeleteAction,
 		},
+		// Should remove (Tags with encoded chars)
+		{
+			inputConfig:    `<LifecycleConfiguration><Rule><Filter><And><Tag><Key>factory</Key><Value>true</Value></Tag><Tag><Key>store forever</Key><Value>false</Value></Tag></And></Filter><Status>Enabled</Status><Expiration><Date>` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + `</Date></Expiration></Rule></LifecycleConfiguration>`,
+			objectName:     "fooobject",
+			objectTags:     "store+forever=false&factory=true",
+			objectModTime:  time.Now().UTC().Add(-24 * time.Hour), // Created 1 day ago
+			expectedAction: DeleteAction,
+		},
 
 		// Should not remove (Tags don't match)
 		{