diff --git a/cmd/api-errors.go b/cmd/api-errors.go index 33f75f6aa..b70437193 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -32,6 +32,7 @@ import ( "github.com/minio/minio/cmd/crypto" "github.com/minio/minio/cmd/logger" "github.com/minio/minio/pkg/auth" + "github.com/minio/minio/pkg/bucket/lifecycle" objectlock "github.com/minio/minio/pkg/bucket/object/lock" "github.com/minio/minio/pkg/bucket/object/tagging" "github.com/minio/minio/pkg/bucket/policy" @@ -1795,6 +1796,12 @@ func toAPIError(ctx context.Context, err error) APIError { // their internal error types. This code is only // useful with gateway implementations. switch e := err.(type) { + case lifecycle.Error: + apiErr = APIError{ + Code: "InvalidRequest", + Description: e.Error(), + HTTPStatusCode: http.StatusBadRequest, + } case tagging.Error: apiErr = APIError{ Code: "InvalidTag", diff --git a/cmd/bucket-lifecycle-handler.go b/cmd/bucket-lifecycle-handler.go index 969dccd16..7161baf9f 100644 --- a/cmd/bucket-lifecycle-handler.go +++ b/cmd/bucket-lifecycle-handler.go @@ -67,7 +67,7 @@ func (api objectAPIHandlers) PutBucketLifecycleHandler(w http.ResponseWriter, r bucketLifecycle, err := lifecycle.ParseLifecycleConfig(io.LimitReader(r.Body, r.ContentLength)) if err != nil { - writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMalformedXML), r.URL, guessIsBrowserReq(r)) + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) return } diff --git a/cmd/daily-lifecycle-ops.go b/cmd/daily-lifecycle-ops.go index 9f2c9141b..78d3df397 100644 --- a/cmd/daily-lifecycle-ops.go +++ b/cmd/daily-lifecycle-ops.go @@ -129,7 +129,7 @@ func lifecycleRound(ctx context.Context, objAPI ObjectLayer) error { // Calculate the common prefix of all lifecycle rules var prefixes []string for _, rule := range l.Rules { - prefixes = append(prefixes, rule.Filter.Prefix) + prefixes = append(prefixes, rule.Prefix()) } commonPrefix := lcp(prefixes) @@ -143,7 +143,7 @@ func lifecycleRound(ctx context.Context, objAPI ObjectLayer) error { var objects []string for _, obj := range res.Objects { // Find the action that need to be executed - action := l.ComputeAction(obj.Name, obj.ModTime) + action := l.ComputeAction(obj.Name, obj.UserTags, obj.ModTime) switch action { case lifecycle.DeleteAction: objects = append(objects, obj.Name) diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 6a5a300bb..653b41b60 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -2877,7 +2877,6 @@ func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *h } tagging, err := tagging.ParseTagging(io.LimitReader(r.Body, r.ContentLength)) - if err != nil { writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) return diff --git a/pkg/bucket/lifecycle/and.go b/pkg/bucket/lifecycle/and.go index c91018f7a..508cc6970 100644 --- a/pkg/bucket/lifecycle/and.go +++ b/pkg/bucket/lifecycle/and.go @@ -18,25 +18,47 @@ package lifecycle import ( "encoding/xml" - "errors" + + "github.com/minio/minio/pkg/bucket/object/tagging" ) // 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 []Tag `xml:"Tag,omitempty"` + XMLName xml.Name `xml:"And"` + Prefix string `xml:"Prefix,omitempty"` + Tags []tagging.Tag `xml:"Tag,omitempty"` } -var errAndUnsupported = errors.New("Specifying tag is not supported") +var errDuplicateTagKey = Errorf("Duplicate Tag Keys are not allowed") -// UnmarshalXML is extended to indicate lack of support for And xml -// tag in object lifecycle configuration -func (a And) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { - return errAndUnsupported +// isEmpty returns true if Tags field is null +func (a And) isEmpty() bool { + return len(a.Tags) == 0 && a.Prefix == "" } -// MarshalXML is extended to leave out tags -func (a And) MarshalXML(e *xml.Encoder, start xml.StartElement) error { +// Validate - validates the And field +func (a And) Validate() error { + if a.ContainsDuplicateTag() { + return errDuplicateTagKey + } + for _, t := range a.Tags { + if err := t.Validate(); err != nil { + return err + } + } return nil } + +// ContainsDuplicateTag - returns true if duplicate keys are present in And +func (a And) ContainsDuplicateTag() bool { + x := make(map[string]struct{}, len(a.Tags)) + + for _, t := range a.Tags { + if _, has := x[t.Key]; has { + return true + } + x[t.Key] = struct{}{} + } + + return false +} diff --git a/pkg/bucket/lifecycle/error.go b/pkg/bucket/lifecycle/error.go new file mode 100644 index 000000000..31138ad15 --- /dev/null +++ b/pkg/bucket/lifecycle/error.go @@ -0,0 +1,44 @@ +/* + * MinIO Cloud Storage, (C) 2020 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package lifecycle + +import ( + "fmt" +) + +// Error is the generic type for any error happening during tag +// parsing. +type Error struct { + err error +} + +// Errorf - formats according to a format specifier and returns +// the string as a value that satisfies error of type tagging.Error +func Errorf(format string, a ...interface{}) error { + return Error{err: fmt.Errorf(format, a...)} +} + +// Unwrap the internal error. +func (e Error) Unwrap() error { return e.err } + +// Error 'error' compatible method. +func (e Error) Error() string { + if e.err == nil { + return "lifecycle: cause " + } + return e.err.Error() +} diff --git a/pkg/bucket/lifecycle/expiration.go b/pkg/bucket/lifecycle/expiration.go index 3e10a8823..961b41c9a 100644 --- a/pkg/bucket/lifecycle/expiration.go +++ b/pkg/bucket/lifecycle/expiration.go @@ -18,15 +18,14 @@ package lifecycle import ( "encoding/xml" - "errors" "time" ) var ( - errLifecycleInvalidDate = errors.New("Date must be provided in ISO 8601 format") - errLifecycleInvalidDays = errors.New("Days must be positive integer when used with Expiration") - errLifecycleInvalidExpiration = errors.New("At least one of Days or Date should be present inside Expiration") - errLifecycleDateNotMidnight = errors.New(" 'Date' must be at midnight GMT") + errLifecycleInvalidDate = Errorf("Date must be provided in ISO 8601 format") + errLifecycleInvalidDays = Errorf("Days must be positive integer when used with Expiration") + errLifecycleInvalidExpiration = Errorf("At least one of Days or Date should be present inside Expiration") + errLifecycleDateNotMidnight = Errorf("'Date' must be at midnight GMT") ) // ExpirationDays is a type alias to unmarshal Days in Expiration @@ -121,7 +120,6 @@ func (e Expiration) Validate() error { // IsDaysNull returns true if days field is null func (e Expiration) IsDaysNull() bool { return e.Days == ExpirationDays(0) - } // IsDateNull returns true if date field is null diff --git a/pkg/bucket/lifecycle/filter.go b/pkg/bucket/lifecycle/filter.go index 7f370e096..3817aae42 100644 --- a/pkg/bucket/lifecycle/filter.go +++ b/pkg/bucket/lifecycle/filter.go @@ -16,17 +16,52 @@ package lifecycle -import "encoding/xml" +import ( + "encoding/xml" + + "github.com/minio/minio/pkg/bucket/object/tagging" +) // Filter - a filter for a lifecycle configuration Rule. type Filter struct { - XMLName xml.Name `xml:"Filter"` - And And `xml:"And,omitempty"` - Prefix string `xml:"Prefix"` - Tag Tag `xml:"Tag,omitempty"` + XMLName xml.Name `xml:"Filter"` + Prefix string `xml:"Prefix,omitempty"` + And And `xml:"And,omitempty"` + Tag tagging.Tag `xml:"Tag,omitempty"` } +var ( + errInvalidFilter = Errorf("Filter must have exactly one of Prefix, Tag, or And specified") +) + // Validate - validates the filter element func (f Filter) Validate() error { + // A Filter must have exactly one of Prefix, Tag, or And specified. + if !f.And.isEmpty() { + if f.Prefix != "" { + return errInvalidFilter + } + if !f.Tag.IsEmpty() { + return errInvalidFilter + } + if err := f.And.Validate(); err != nil { + return err + } + } + if f.Prefix != "" { + if !f.Tag.IsEmpty() { + return errInvalidFilter + } + } + if !f.Tag.IsEmpty() { + if err := f.Tag.Validate(); err != nil { + return err + } + } return nil } + +// isEmpty - returns true if Filter tag is empty +func (f Filter) isEmpty() bool { + return f.And.isEmpty() && f.Prefix == "" && f.Tag == tagging.Tag{} +} diff --git a/pkg/bucket/lifecycle/filter_test.go b/pkg/bucket/lifecycle/filter_test.go index 9077195fa..a398fce3d 100644 --- a/pkg/bucket/lifecycle/filter_test.go +++ b/pkg/bucket/lifecycle/filter_test.go @@ -31,23 +31,91 @@ func TestUnsupportedFilters(t *testing.T) { }{ { // Filter with And tags inputXML: ` - - - - `, - expectedErr: errAndUnsupported, + + key-prefix + + `, + expectedErr: nil, }, { // Filter with Tag tags inputXML: ` - - `, - expectedErr: errTagUnsupported, + + key1 + value1 + + `, + expectedErr: nil, + }, + { // Filter with Prefix tag + inputXML: ` + key-prefix + `, + expectedErr: nil, + }, + { // Filter without And and multiple Tag tags + inputXML: ` + key-prefix + + key1 + value1 + + + key2 + value2 + + `, + expectedErr: errInvalidFilter, + }, + { // Filter with And, Prefix & multiple Tag tags + inputXML: ` + + key-prefix + + key1 + value1 + + + key2 + value2 + + + `, + expectedErr: nil, + }, + { // Filter with And and multiple Tag tags + inputXML: ` + + + key1 + value1 + + + key2 + value2 + + + `, + expectedErr: nil, + }, + { // Filter without And and single Tag tag + inputXML: ` + key-prefix + + key1 + value1 + + `, + expectedErr: errInvalidFilter, }, } for i, tc := range testCases { t.Run(fmt.Sprintf("Test %d", i+1), func(t *testing.T) { var filter Filter err := xml.Unmarshal([]byte(tc.inputXML), &filter) + if err != nil { + t.Fatalf("%d: Expected no error but got %v", i+1, err) + } + err = filter.Validate() if err != tc.expectedErr { t.Fatalf("%d: Expected %v but got %v", i+1, tc.expectedErr, err) } diff --git a/pkg/bucket/lifecycle/lifecycle.go b/pkg/bucket/lifecycle/lifecycle.go index 4540aebc5..18113f631 100644 --- a/pkg/bucket/lifecycle/lifecycle.go +++ b/pkg/bucket/lifecycle/lifecycle.go @@ -18,16 +18,15 @@ package lifecycle import ( "encoding/xml" - "errors" "io" "strings" "time" ) var ( - errLifecycleTooManyRules = errors.New("Lifecycle configuration allows a maximum of 1000 rules") - errLifecycleNoRule = errors.New("Lifecycle configuration should have at least one rule") - errLifecycleOverlappingPrefix = errors.New("Lifecycle configuration has rules with overlapping prefix") + errLifecycleTooManyRules = Errorf("Lifecycle configuration allows a maximum of 1000 rules") + errLifecycleNoRule = Errorf("Lifecycle configuration should have at least one rule") + errLifecycleOverlappingPrefix = Errorf("Lifecycle configuration has rules with overlapping prefix") ) // Action represents a delete action or other transition @@ -88,8 +87,8 @@ func (lc Lifecycle) Validate() error { // N B Empty prefixes overlap with all prefixes otherRules := lc.Rules[i+1:] for _, otherRule := range otherRules { - if strings.HasPrefix(lc.Rules[i].Filter.Prefix, otherRule.Filter.Prefix) || - strings.HasPrefix(otherRule.Filter.Prefix, lc.Rules[i].Filter.Prefix) { + if strings.HasPrefix(lc.Rules[i].Prefix(), otherRule.Prefix()) || + strings.HasPrefix(otherRule.Prefix(), lc.Rules[i].Prefix()) { return errLifecycleOverlappingPrefix } } @@ -99,13 +98,20 @@ func (lc Lifecycle) Validate() error { // FilterRuleActions returns the expiration and transition from the object name // after evaluating all rules. -func (lc Lifecycle) FilterRuleActions(objName string) (Expiration, Transition) { +func (lc Lifecycle) FilterRuleActions(objName, objTags string) (Expiration, Transition) { for _, rule := range lc.Rules { if strings.ToLower(rule.Status) != "enabled" { continue } - if strings.HasPrefix(objName, rule.Filter.Prefix) { - return rule.Expiration, Transition{} + tags := rule.Tags() + if strings.HasPrefix(objName, rule.Prefix()) { + if tags != "" { + if strings.Contains(objTags, tags) { + return rule.Expiration, Transition{} + } + } else { + return rule.Expiration, Transition{} + } } } return Expiration{}, Transition{} @@ -113,9 +119,9 @@ func (lc Lifecycle) FilterRuleActions(objName string) (Expiration, Transition) { // ComputeAction returns the action to perform by evaluating all lifecycle rules // against the object name and its modification time. -func (lc Lifecycle) ComputeAction(objName string, modTime time.Time) Action { +func (lc Lifecycle) ComputeAction(objName, objTags string, modTime time.Time) Action { var action = NoneAction - exp, _ := lc.FilterRuleActions(objName) + exp, _ := lc.FilterRuleActions(objName, objTags) if !exp.IsDateNull() { if time.Now().After(exp.Date.Time) { action = DeleteAction diff --git a/pkg/bucket/lifecycle/lifecycle_test.go b/pkg/bucket/lifecycle/lifecycle_test.go index 5c0a9b6fc..035eb64d9 100644 --- a/pkg/bucket/lifecycle/lifecycle_test.go +++ b/pkg/bucket/lifecycle/lifecycle_test.go @@ -52,7 +52,9 @@ func TestParseLifecycleConfig(t *testing.T) { Status: "Enabled", Expiration: Expiration{Days: ExpirationDays(3)}, Filter: Filter{ - Prefix: "/a/b/c", + And: And{ + Prefix: "/a/b/c", + }, }, } overlappingRules := []Rule{rule1, rule2} @@ -67,26 +69,26 @@ func TestParseLifecycleConfig(t *testing.T) { }{ { // Valid lifecycle config inputConfig: ` - - - prefix - - Enabled - 3 - - - - another-prefix - - Enabled - 3 - - `, + + + prefix + + Enabled + 3 + + + + another-prefix + + Enabled + 3 + + `, expectedErr: nil, }, { // lifecycle config with no rules inputConfig: ` - `, + `, expectedErr: errLifecycleNoRule, }, { // lifecycle config with more than 1000 rules @@ -105,9 +107,7 @@ func TestParseLifecycleConfig(t *testing.T) { if _, err = ParseLifecycleConfig(bytes.NewReader([]byte(tc.inputConfig))); err != tc.expectedErr { t.Fatalf("%d: Expected %v but got %v", i+1, tc.expectedErr, err) } - }) - } } @@ -163,6 +163,7 @@ func TestComputeActions(t *testing.T) { testCases := []struct { inputConfig string objectName string + objectTags string objectModTime time.Time expectedAction Action }{ @@ -213,6 +214,46 @@ func TestComputeActions(t *testing.T) { objectModTime: time.Now().UTC().Add(-24 * time.Hour), // Created 1 day ago expectedAction: DeleteAction, }, + // Should remove (Tags match) + { + inputConfig: `foodir/tag1value1Enabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + ``, + objectName: "foodir/fooobject", + objectTags: "tag1=value1&tag2=value2", + objectModTime: time.Now().UTC().Add(-24 * time.Hour), // Created 1 day ago + expectedAction: DeleteAction, + }, + // Should remove (Multiple Rules, Tags match) + { + inputConfig: `foodir/tag1value1tag2value2Enabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + `abc/tag2valueEnabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + ``, + objectName: "foodir/fooobject", + objectTags: "tag1=value1&tag2=value2", + objectModTime: time.Now().UTC().Add(-24 * time.Hour), // Created 1 day ago + expectedAction: DeleteAction, + }, + // Should remove (Tags match) + { + inputConfig: `foodir/tag1value1tag2value2Enabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + ``, + objectName: "foodir/fooobject", + objectTags: "tag1=value1&tag2=value2", + objectModTime: time.Now().UTC().Add(-24 * time.Hour), // Created 1 day ago + expectedAction: DeleteAction, + }, + // Should not remove (Tags don't match) + { + inputConfig: `foodir/tagvalue1Enabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + ``, + objectName: "foodir/fooobject", + objectTags: "tag1=value1", + objectModTime: time.Now().UTC().Add(-24 * time.Hour), // Created 1 day ago + expectedAction: NoneAction, + }, + // Should not remove (Tags match, but prefix doesn't match) + { + inputConfig: `foodir/tag1value1Enabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + ``, + objectName: "foxdir/fooobject", + objectTags: "tag1=value1", + objectModTime: time.Now().UTC().Add(-24 * time.Hour), // Created 1 day ago + expectedAction: NoneAction, + }, } for i, tc := range testCases { @@ -221,7 +262,7 @@ func TestComputeActions(t *testing.T) { if err != nil { t.Fatalf("%d: Got unexpected error: %v", i+1, err) } - if resultAction := lc.ComputeAction(tc.objectName, tc.objectModTime); resultAction != tc.expectedAction { + if resultAction := lc.ComputeAction(tc.objectName, tc.objectTags, tc.objectModTime); resultAction != tc.expectedAction { t.Fatalf("%d: Expected action: `%v`, got: `%v`", i+1, tc.expectedAction, resultAction) } }) diff --git a/pkg/bucket/lifecycle/noncurrentversion.go b/pkg/bucket/lifecycle/noncurrentversion.go index 15eb43b03..d879c12c4 100644 --- a/pkg/bucket/lifecycle/noncurrentversion.go +++ b/pkg/bucket/lifecycle/noncurrentversion.go @@ -18,7 +18,6 @@ package lifecycle import ( "encoding/xml" - "errors" ) // NoncurrentVersionExpiration - an action for lifecycle configuration rule. @@ -34,8 +33,8 @@ type NoncurrentVersionTransition struct { } var ( - errNoncurrentVersionExpirationUnsupported = errors.New("Specifying is not supported") - errNoncurrentVersionTransitionUnsupported = errors.New("Specifying is not supported") + errNoncurrentVersionExpirationUnsupported = Errorf("Specifying is not supported") + errNoncurrentVersionTransitionUnsupported = Errorf("Specifying is not supported") ) // UnmarshalXML is extended to indicate lack of support for diff --git a/pkg/bucket/lifecycle/rule.go b/pkg/bucket/lifecycle/rule.go index 3d55058fc..b99d7caae 100644 --- a/pkg/bucket/lifecycle/rule.go +++ b/pkg/bucket/lifecycle/rule.go @@ -17,8 +17,8 @@ package lifecycle import ( + "bytes" "encoding/xml" - "errors" ) // Rule - a rule for lifecycle configuration. @@ -26,7 +26,7 @@ type Rule struct { XMLName xml.Name `xml:"Rule"` ID string `xml:"ID,omitempty"` Status string `xml:"Status"` - Filter Filter `xml:"Filter"` + Filter Filter `xml:"Filter,omitempty"` Expiration Expiration `xml:"Expiration,omitempty"` Transition Transition `xml:"Transition,omitempty"` // FIXME: add a type to catch unsupported AbortIncompleteMultipartUpload AbortIncompleteMultipartUpload `xml:"AbortIncompleteMultipartUpload,omitempty"` @@ -35,13 +35,13 @@ type Rule struct { } var ( - errInvalidRuleID = errors.New("ID must be less than 255 characters") - errEmptyRuleStatus = errors.New("Status should not be empty") - errInvalidRuleStatus = errors.New("Status must be set to either Enabled or Disabled") - errMissingExpirationAction = errors.New("No expiration action found") + errInvalidRuleID = Errorf("ID must be less than 255 characters") + errEmptyRuleStatus = Errorf("Status should not be empty") + errInvalidRuleStatus = Errorf("Status must be set to either Enabled or Disabled") + errMissingExpirationAction = Errorf("No expiration action found") ) -// isIDValid - checks if ID is valid or not. +// validateID - checks if ID is valid or not. func (r Rule) validateID() error { // cannot be longer than 255 characters if len(string(r.ID)) > 255 { @@ -50,7 +50,7 @@ func (r Rule) validateID() error { return nil } -// isStatusValid - checks if status is valid or not. +// validateStatus - checks if status is valid or not. func (r Rule) validateStatus() error { // Status can't be empty if len(r.Status) == 0 { @@ -71,6 +71,43 @@ func (r Rule) validateAction() error { return nil } +func (r Rule) validateFilter() error { + return r.Filter.Validate() +} + +// Prefix - a rule can either have prefix under or under +// . This method returns the prefix from the +// location where it is available +func (r Rule) Prefix() string { + if r.Filter.Prefix != "" { + return r.Filter.Prefix + } + if r.Filter.And.Prefix != "" { + return r.Filter.And.Prefix + } + return "" +} + +// Tags - a rule can either have tag under or under +// . This method returns all the tags from the +// rule in the format tag1=value1&tag2=value2 +func (r Rule) Tags() string { + if !r.Filter.Tag.IsEmpty() { + return r.Filter.Tag.String() + } + if len(r.Filter.And.Tags) != 0 { + var buf bytes.Buffer + for _, t := range r.Filter.And.Tags { + if buf.Len() > 0 { + buf.WriteString("&") + } + buf.WriteString(t.String()) + } + return buf.String() + } + return "" +} + // Validate - validates the rule element func (r Rule) Validate() error { if err := r.validateID(); err != nil { @@ -82,5 +119,8 @@ func (r Rule) Validate() error { if err := r.validateAction(); err != nil { return err } + if err := r.validateFilter(); err != nil { + return err + } return nil } diff --git a/pkg/bucket/lifecycle/tag.go b/pkg/bucket/lifecycle/tag.go index 580c1c021..f3c9bcaed 100644 --- a/pkg/bucket/lifecycle/tag.go +++ b/pkg/bucket/lifecycle/tag.go @@ -18,7 +18,6 @@ package lifecycle import ( "encoding/xml" - "errors" ) // Tag - a tag for a lifecycle configuration Rule filter. @@ -28,7 +27,7 @@ type Tag struct { Value string `xml:"Value,omitempty"` } -var errTagUnsupported = errors.New("Specifying is not supported") +var errTagUnsupported = Errorf("Specifying is not supported") // UnmarshalXML is extended to indicate lack of support for Tag // xml tag in object lifecycle configuration diff --git a/pkg/bucket/lifecycle/transition.go b/pkg/bucket/lifecycle/transition.go index 161e7a4e1..7937b34cf 100644 --- a/pkg/bucket/lifecycle/transition.go +++ b/pkg/bucket/lifecycle/transition.go @@ -18,7 +18,6 @@ package lifecycle import ( "encoding/xml" - "errors" ) // Transition - transition actions for a rule in lifecycle configuration. @@ -29,7 +28,7 @@ type Transition struct { StorageClass string `xml:"StorageClass"` } -var errTransitionUnsupported = errors.New("Specifying tag is not supported") +var errTransitionUnsupported = Errorf("Specifying tag is not supported") // UnmarshalXML is extended to indicate lack of support for Transition // xml tag in object lifecycle configuration diff --git a/pkg/bucket/object/tagging/tag.go b/pkg/bucket/object/tagging/tag.go index 5c59851de..2f2faa726 100644 --- a/pkg/bucket/object/tagging/tag.go +++ b/pkg/bucket/object/tagging/tag.go @@ -18,14 +18,15 @@ package tagging import ( "encoding/xml" + "strings" "unicode/utf8" ) // Tag - single tag type Tag struct { XMLName xml.Name `xml:"Tag"` - Key string `xml:"Key"` - Value string `xml:"Value"` + Key string `xml:"Key,omitempty"` + Value string `xml:"Value,omitempty"` } // Validate - validates the tag element @@ -49,6 +50,10 @@ func (t Tag) validateKey() error { if len(t.Key) == 0 { return ErrInvalidTagKey } + // Tag key shouldn't have "&" + if strings.Contains(t.Key, "&") { + return ErrInvalidTagKey + } return nil } @@ -58,5 +63,20 @@ func (t Tag) validateValue() error { if utf8.RuneCountInString(t.Value) > maxTagValueLength { return ErrInvalidTagValue } + // Tag value shouldn't have "&" + if strings.Contains(t.Value, "&") { + return ErrInvalidTagValue + } return nil } + +// IsEmpty - checks if tag is empty or not +func (t Tag) IsEmpty() bool { + return t.Key == "" && t.Value == "" +} + +// String - returns a string in format "tag1=value1" for the +// current Tag +func (t Tag) String() string { + return t.Key + "=" + t.Value +} diff --git a/pkg/bucket/object/tagging/tagging.go b/pkg/bucket/object/tagging/tagging.go index 9fc683ecd..004cc470f 100644 --- a/pkg/bucket/object/tagging/tagging.go +++ b/pkg/bucket/object/tagging/tagging.go @@ -51,11 +51,11 @@ func (t Tagging) Validate() error { if len(t.TagSet.Tags) > maxTags { return ErrTooManyTags } + if t.TagSet.ContainsDuplicateTag() { + return ErrInvalidTag + } // Validate all the rules in the tagging config for _, ts := range t.TagSet.Tags { - if t.TagSet.ContainsDuplicate(ts.Key) { - return ErrInvalidTag - } if err := ts.Validate(); err != nil { return err } @@ -71,8 +71,7 @@ func (t Tagging) String() string { if buf.Len() > 0 { buf.WriteString("&") } - buf.WriteString(tag.Key + "=") - buf.WriteString(tag.Value) + buf.WriteString(tag.String()) } return buf.String() } diff --git a/pkg/bucket/object/tagging/tagset.go b/pkg/bucket/object/tagging/tagset.go index 135890a30..b4997db95 100644 --- a/pkg/bucket/object/tagging/tagset.go +++ b/pkg/bucket/object/tagging/tagset.go @@ -26,16 +26,16 @@ type TagSet struct { Tags []Tag `xml:"Tag"` } -// ContainsDuplicate - returns true if duplicate keys are present in TagSet -func (t TagSet) ContainsDuplicate(key string) bool { - var found bool - for _, tag := range t.Tags { - if tag.Key == key { - if found { - return true - } - found = true +// ContainsDuplicateTag - returns true if duplicate keys are present in TagSet +func (t TagSet) ContainsDuplicateTag() bool { + x := make(map[string]struct{}, len(t.Tags)) + + for _, t := range t.Tags { + if _, has := x[t.Key]; has { + return true } + x[t.Key] = struct{}{} } + return false }