relax validation when loading lifecycle document from the backend (#9612)

cmd/bucket-lifecycle-handler.go

@@ -72,6 +72,12 @@ func (api objectAPIHandlers) PutBucketLifecycleHandler(w http.ResponseWriter, r
 		return
 	}
 
+	// Validate the received bucket policy document
+	if err = bucketLifecycle.Validate(); err != nil {
+		writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
+		return
+	}
+
 	if err = objAPI.SetBucketLifecycle(ctx, bucket, bucketLifecycle); err != nil {
 		writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
 		return

cmd/bucket-lifecycle.go

@@ -23,6 +23,7 @@ import (
 	"path"
 	"sync"
 
+	"github.com/minio/minio/cmd/logger"
 	"github.com/minio/minio/pkg/bucket/lifecycle"
 )
 
@@ -148,9 +149,15 @@ func (sys *LifecycleSys) load(buckets []BucketInfo, objAPI ObjectLayer) error {
 			return err
 		}
 
+		// Do not load the lifecycle configuration if it is not valid
+		err = config.Validate()
+		if err != nil {
+			logger.LogIf(context.Background(), err)
+			continue
+		}
+
 		sys.Set(bucket.Name, config)
 	}
-
 	return nil
 }
 

pkg/bucket/lifecycle/and.go

@@ -18,15 +18,13 @@ package lifecycle
 
 import (
 	"encoding/xml"
-
-	"github.com/minio/minio-go/v6/pkg/tags"
 )
 
 // And - a tag to combine a prefix and multiple tags for lifecycle configuration rule.
 type And struct {
-	XMLName xml.Name   `xml:"And"`
-	Prefix  string     `xml:"Prefix,omitempty"`
-	Tags    []tags.Tag `xml:"Tag,omitempty"`
+	XMLName xml.Name `xml:"And"`
+	Prefix  string   `xml:"Prefix,omitempty"`
+	Tags    []Tag    `xml:"Tag,omitempty"`
 }
 
 var errDuplicateTagKey = Errorf("Duplicate Tag Keys are not allowed")

pkg/bucket/lifecycle/filter.go

@@ -18,8 +18,6 @@ package lifecycle
 
 import (
 	"encoding/xml"
-
-	"github.com/minio/minio-go/v6/pkg/tags"
 )
 
 var (
@@ -31,7 +29,7 @@ type Filter struct {
 	XMLName xml.Name `xml:"Filter"`
 	Prefix  string
 	And     And
-	Tag     tags.Tag
+	Tag     Tag
 }
 
 // MarshalXML - produces the xml representation of the Filter struct

pkg/bucket/lifecycle/lifecycle.go

@@ -57,9 +57,6 @@ func ParseLifecycleConfig(reader io.Reader) (*Lifecycle, error) {
 	if err := xml.NewDecoder(reader).Decode(&lc); err != nil {
 		return nil, err
 	}
-	if err := lc.Validate(); err != nil {
-		return nil, err
-	}
 	return &lc, nil
 }
 

pkg/bucket/lifecycle/lifecycle_test.go

@@ -24,7 +24,7 @@ import (
 	"time"
 )
 
-func TestParseLifecycleConfig(t *testing.T) {
+func TestParseAndValidateLifecycleConfig(t *testing.T) {
 	// Test for  lifecycle config with more than 1000 rules
 	var manyRules []Rule
 	rule := Rule{
@@ -64,8 +64,9 @@ func TestParseLifecycleConfig(t *testing.T) {
 	}
 
 	testCases := []struct {
-		inputConfig string
-		expectedErr error
+		inputConfig           string
+		expectedParsingErr    error
+		expectedValidationErr error
 	}{
 		{ // Valid lifecycle config
 			inputConfig: `<LifecycleConfiguration>
@@ -84,28 +85,36 @@ func TestParseLifecycleConfig(t *testing.T) {
 		                          <Expiration><Days>3</Days></Expiration>
 		                          </Rule>
 		                          </LifecycleConfiguration>`,
-			expectedErr: nil,
+			expectedParsingErr:    nil,
+			expectedValidationErr: nil,
 		},
 		{ // lifecycle config with no rules
 			inputConfig: `<LifecycleConfiguration>
 		                          </LifecycleConfiguration>`,
-			expectedErr: errLifecycleNoRule,
+			expectedParsingErr:    nil,
+			expectedValidationErr: errLifecycleNoRule,
 		},
 		{ // lifecycle config with more than 1000 rules
-			inputConfig: string(manyRuleLcConfig),
-			expectedErr: errLifecycleTooManyRules,
+			inputConfig:           string(manyRuleLcConfig),
+			expectedParsingErr:    nil,
+			expectedValidationErr: errLifecycleTooManyRules,
 		},
 		{ // lifecycle config with rules having overlapping prefix
-			inputConfig: string(overlappingLcConfig),
-			expectedErr: errLifecycleOverlappingPrefix,
+			inputConfig:           string(overlappingLcConfig),
+			expectedParsingErr:    nil,
+			expectedValidationErr: errLifecycleOverlappingPrefix,
 		},
 	}
 
 	for i, tc := range testCases {
 		t.Run(fmt.Sprintf("Test %d", i+1), func(t *testing.T) {
-			var err error
-			if _, err = ParseLifecycleConfig(bytes.NewReader([]byte(tc.inputConfig))); err != tc.expectedErr {
-				t.Fatalf("%d: Expected %v but got %v", i+1, tc.expectedErr, err)
+			lc, err := ParseLifecycleConfig(bytes.NewReader([]byte(tc.inputConfig)))
+			if err != tc.expectedParsingErr {
+				t.Fatalf("%d: Expected %v during parsing but got %v", i+1, tc.expectedParsingErr, err)
+			}
+			err = lc.Validate()
+			if err != tc.expectedValidationErr {
+				t.Fatalf("%d: Expected %v during parsing but got %v", i+1, tc.expectedValidationErr, err)
 			}
 		})
 	}

pkg/bucket/lifecycle/tag.go

@@ -18,6 +18,7 @@ package lifecycle
 
 import (
 	"encoding/xml"
+	"unicode/utf8"
 )
 
 // Tag - a tag for a lifecycle configuration Rule filter.
@@ -27,15 +28,29 @@ type Tag struct {
 	Value   string   `xml:"Value,omitempty"`
 }
 
-var errTagUnsupported = Errorf("Specifying <Tag></Tag> is not supported")
+var (
+	errInvalidTagKey   = Errorf("The TagKey you have provided is invalid")
+	errInvalidTagValue = Errorf("The TagValue you have provided is invalid")
+)
+
+func (tag Tag) String() string {
+	return tag.Key + "=" + tag.Value
+}
 
-// UnmarshalXML is extended to indicate lack of support for Tag
-// xml tag in object lifecycle configuration
-func (t Tag) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
-	return errTagUnsupported
+// IsEmpty returns whether this tag is empty or not.
+func (tag Tag) IsEmpty() bool {
+	return tag.Key == ""
 }
 
-// MarshalXML is extended to leave out <Tag></Tag> tags
-func (t Tag) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
+// Validate checks this tag.
+func (tag Tag) Validate() error {
+	if len(tag.Key) == 0 || utf8.RuneCountInString(tag.Key) > 128 {
+		return errInvalidTagKey
+	}
+
+	if utf8.RuneCountInString(tag.Value) > 256 {
+		return errInvalidTagValue
+	}
+
 	return nil
 }