diff --git a/pkg/bucket/lifecycle/expiration.go b/pkg/bucket/lifecycle/expiration.go index fd70e45b9..e9ff8cf79 100644 --- a/pkg/bucket/lifecycle/expiration.go +++ b/pkg/bucket/lifecycle/expiration.go @@ -98,36 +98,76 @@ func (eDate ExpirationDate) MarshalXML(e *xml.Encoder, startElement xml.StartEle } // ExpireDeleteMarker represents value of ExpiredObjectDeleteMarker field in Expiration XML element. -type ExpireDeleteMarker bool +type ExpireDeleteMarker struct { + val bool + set bool +} // Expiration - expiration actions for a rule in lifecycle configuration. type Expiration struct { XMLName xml.Name `xml:"Expiration"` Days ExpirationDays `xml:"Days,omitempty"` Date ExpirationDate `xml:"Date,omitempty"` - DeleteMarker ExpireDeleteMarker `xml:"ExpiredObjectDeleteMarker,omitempty"` + DeleteMarker ExpireDeleteMarker `xml:"ExpiredObjectDeleteMarker"` + + set bool } // MarshalXML encodes delete marker boolean into an XML form. func (b ExpireDeleteMarker) MarshalXML(e *xml.Encoder, startElement xml.StartElement) error { - if !b { + if !b.set { + return nil + } + return e.EncodeElement(b.val, startElement) +} + +// UnmarshalXML decodes delete marker boolean from the XML form. +func (b *ExpireDeleteMarker) UnmarshalXML(d *xml.Decoder, startElement xml.StartElement) error { + var exp bool + err := d.DecodeElement(&exp, &startElement) + if err != nil { + return err + } + b.val = exp + b.set = true + return nil +} + +// MarshalXML encodes expiration field into an XML form. +func (e Expiration) MarshalXML(enc *xml.Encoder, startElement xml.StartElement) error { + if !e.set { return nil } - type expireDeleteMarkerWrapper ExpireDeleteMarker - return e.EncodeElement(expireDeleteMarkerWrapper(b), startElement) + type expirationWrapper Expiration + return enc.EncodeElement(expirationWrapper(e), startElement) +} + +// UnmarshalXML decodes expiration field from the XML form. +func (e *Expiration) UnmarshalXML(d *xml.Decoder, startElement xml.StartElement) error { + type expirationWrapper Expiration + var exp expirationWrapper + err := d.DecodeElement(&exp, &startElement) + if err != nil { + return err + } + *e = Expiration(exp) + e.set = true + return nil } // Validate - validates the "Expiration" element func (e Expiration) Validate() error { + if !e.set { + return nil + } + // DeleteMarker cannot be specified if date or dates are specified. - if (!e.IsDateNull() || !e.IsDateNull()) && bool(e.DeleteMarker) { + if (!e.IsDaysNull() || !e.IsDateNull()) && e.DeleteMarker.set { return errLifecycleInvalidDeleteMarker } - // Neither expiration days or date is specified - // if delete marker is false one of them should be specified - if !bool(e.DeleteMarker) && e.IsDaysNull() && e.IsDateNull() { - return errLifecycleInvalidExpiration + if !e.DeleteMarker.set && e.IsDaysNull() && e.IsDateNull() { + return errXMLNotWellFormed } // Both expiration days and date are specified diff --git a/pkg/bucket/lifecycle/expiration_test.go b/pkg/bucket/lifecycle/expiration_test.go index f863f9e1e..24592efe6 100644 --- a/pkg/bucket/lifecycle/expiration_test.go +++ b/pkg/bucket/lifecycle/expiration_test.go @@ -78,7 +78,7 @@ func TestInvalidExpiration(t *testing.T) { { // Expiration with neither number of days nor a date inputXML: ` `, - expectedErr: errLifecycleInvalidExpiration, + expectedErr: errXMLNotWellFormed, }, { // Expiration with both number of days and a date inputXML: ` @@ -87,6 +87,13 @@ func TestInvalidExpiration(t *testing.T) { `, expectedErr: errLifecycleInvalidExpiration, }, + { // Expiration with both ExpiredObjectDeleteMarker and days + inputXML: ` + 3 + false + `, + expectedErr: errLifecycleInvalidDeleteMarker, + }, } for i, tc := range validationTestCases { t.Run(fmt.Sprintf("Test %d", i+1), func(t *testing.T) { @@ -98,7 +105,7 @@ func TestInvalidExpiration(t *testing.T) { err = expiration.Validate() if err != tc.expectedErr { - t.Fatalf("%d: %v", i+1, err) + t.Fatalf("%d: got: %v, expected: %v", i+1, err, tc.expectedErr) } }) } diff --git a/pkg/bucket/lifecycle/lifecycle.go b/pkg/bucket/lifecycle/lifecycle.go index d0eaa679b..807e633f4 100644 --- a/pkg/bucket/lifecycle/lifecycle.go +++ b/pkg/bucket/lifecycle/lifecycle.go @@ -27,6 +27,7 @@ var ( errLifecycleTooManyRules = Errorf("Lifecycle configuration allows a maximum of 1000 rules") errLifecycleNoRule = Errorf("Lifecycle configuration should have at least one rule") errLifecycleDuplicateID = Errorf("Lifecycle configuration has rule with the same ID. Rule ID must be unique.") + errXMLNotWellFormed = Errorf("The XML you provided was not well-formed or did not validate against our published schema") ) // Action represents a delete action or other transition @@ -154,7 +155,7 @@ func (lc Lifecycle) FilterActionableRules(obj ObjectOpts) []Rule { // be expired; if set to false the policy takes no action. This // cannot be specified with Days or Date in a Lifecycle // Expiration Policy. - if rule.Expiration.DeleteMarker { + if rule.Expiration.DeleteMarker.val { rules = append(rules, rule) continue } @@ -193,7 +194,7 @@ func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action { } for _, rule := range lc.FilterActionableRules(obj) { - if obj.DeleteMarker && obj.NumVersions == 1 && bool(rule.Expiration.DeleteMarker) { + if obj.DeleteMarker && obj.NumVersions == 1 && rule.Expiration.DeleteMarker.val { // Indicates whether MinIO will remove a delete marker with no noncurrent versions. // Only latest marker is removed. If set to true, the delete marker will be expired; // if set to false the policy takes no action. This cannot be specified with Days or diff --git a/pkg/bucket/lifecycle/rule_test.go b/pkg/bucket/lifecycle/rule_test.go index 80156712e..12328f9fa 100644 --- a/pkg/bucket/lifecycle/rule_test.go +++ b/pkg/bucket/lifecycle/rule_test.go @@ -62,13 +62,6 @@ func TestInvalidRules(t *testing.T) { inputXML string expectedErr error }{ - { // Rule without expiration action - inputXML: ` - rule without expiration - Enabled - `, - expectedErr: errLifecycleInvalidExpiration, - }, { // Rule with ID longer than 255 characters inputXML: ` babababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababab