fix test_object_lock_put_obj_lock_invalid_days

3 files changed, 66 insertions, 55 deletions

diff --git a/weed/s3api/object_lock_utils.go b/weed/s3api/object_lock_utils.go
index ffde5bd36..e207ca86d 100644
--- a/weed/s3api/object_lock_utils.go
+++ b/weed/s3api/object_lock_utils.go
@@ -59,9 +59,11 @@ func CreateObjectLockConfiguration(enabled bool, mode string, days int, years in
 	if mode != "" && (days > 0 || years > 0) {
 		config.Rule = &ObjectLockRule{
 			DefaultRetention: &DefaultRetention{
-				Mode:  mode,
-				Days:  days,
-				Years: years,
+				Mode:     mode,
+				Days:     days,
+				Years:    years,
+				DaysSet:  days > 0,
+				YearsSet: years > 0,
 			},
 		}
 	}
@@ -106,12 +108,12 @@ func StoreObjectLockConfigurationInExtended(entry *filer_pb.Entry, config *Objec
 		}
 
 		// Store days
-		if defaultRetention.Days > 0 {
+		if defaultRetention.DaysSet && defaultRetention.Days > 0 {
 			entry.Extended[s3_constants.ExtObjectLockDefaultDaysKey] = []byte(strconv.Itoa(defaultRetention.Days))
 		}
 
 		// Store years
-		if defaultRetention.Years > 0 {
+		if defaultRetention.YearsSet && defaultRetention.Years > 0 {
 			entry.Extended[s3_constants.ExtObjectLockDefaultYearsKey] = []byte(strconv.Itoa(defaultRetention.Years))
 		}
 	} else {
@@ -167,9 +169,11 @@ func LoadObjectLockConfigurationFromExtended(entry *filer_pb.Entry) (*ObjectLock
 		if mode != "" && (days > 0 || years > 0) {
 			config.Rule = &ObjectLockRule{
 				DefaultRetention: &DefaultRetention{
-					Mode:  mode,
-					Days:  days,
-					Years: years,
+					Mode:     mode,
+					Days:     days,
+					Years:    years,
+					DaysSet:  days > 0,
+					YearsSet: years > 0,
 				},
 			}
 		}
@@ -192,8 +196,11 @@ func ExtractObjectLockInfoFromConfig(config *ObjectLockConfiguration) (bool, str
 	defaultRetention := config.Rule.DefaultRetention
 
 	// Convert years to days for consistent representation
-	days := defaultRetention.Days
-	if defaultRetention.Years > 0 {
+	days := 0
+	if defaultRetention.DaysSet {
+		days = defaultRetention.Days
+	}
+	if defaultRetention.YearsSet && defaultRetention.Years > 0 {
 		days += defaultRetention.Years * 365
 	}
 
diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go
index 50067f85e..049e0e4ae 100644
--- a/weed/s3api/s3api_object_handlers_put.go
+++ b/weed/s3api/s3api_object_handlers_put.go
@@ -469,7 +469,7 @@ func (s3a *S3ApiServer) applyBucketDefaultRetention(bucket string, entry *filer_
 		return fmt.Errorf("default retention missing mode")
 	}
 
-	if defaultRetention.Days == 0 && defaultRetention.Years == 0 {
+	if !defaultRetention.DaysSet && !defaultRetention.YearsSet {
 		return fmt.Errorf("default retention missing period")
 	}
 
@@ -477,9 +477,9 @@ func (s3a *S3ApiServer) applyBucketDefaultRetention(bucket string, entry *filer_
 	var retainUntilDate time.Time
 	now := time.Now()
 
-	if defaultRetention.Days > 0 {
+	if defaultRetention.DaysSet && defaultRetention.Days > 0 {
 		retainUntilDate = now.AddDate(0, 0, defaultRetention.Days)
-	} else if defaultRetention.Years > 0 {
+	} else if defaultRetention.YearsSet && defaultRetention.Years > 0 {
 		retainUntilDate = now.AddDate(defaultRetention.Years, 0, 0)
 	}
 
@@ -652,7 +652,7 @@ func mapValidationErrorToS3Error(err error) s3err.ErrorCode {
 	case errors.Is(err, ErrRetentionMissingRetainUntilDate):
 		return s3err.ErrInvalidRequest
 	case errors.Is(err, ErrInvalidRetentionModeValue):
-		return s3err.ErrInvalidRequest
+		return s3err.ErrMalformedXML
 	}
 
 	return s3err.ErrInvalidRequest
diff --git a/weed/s3api/s3api_object_retention.go b/weed/s3api/s3api_object_retention.go
index 425a853f7..b3b7635cc 100644
--- a/weed/s3api/s3api_object_retention.go
+++ b/weed/s3api/s3api_object_retention.go
@@ -51,59 +51,66 @@ const (
 // ObjectRetention represents S3 Object Retention configuration
 type ObjectRetention struct {
 	XMLName         xml.Name   `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Retention"`
-	Mode            string     `xml:"Mode,omitempty"`
-	RetainUntilDate *time.Time `xml:"RetainUntilDate,omitempty"`
+	Mode            string     `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Mode,omitempty"`
+	RetainUntilDate *time.Time `xml:"http://s3.amazonaws.com/doc/2006-03-01/ RetainUntilDate,omitempty"`
 }
 
 // ObjectLegalHold represents S3 Object Legal Hold configuration
 type ObjectLegalHold struct {
 	XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ LegalHold"`
-	Status  string   `xml:"Status,omitempty"`
+	Status  string   `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Status,omitempty"`
 }
 
 // ObjectLockConfiguration represents S3 Object Lock Configuration
 type ObjectLockConfiguration struct {
 	XMLName           xml.Name        `xml:"http://s3.amazonaws.com/doc/2006-03-01/ ObjectLockConfiguration"`
-	ObjectLockEnabled string          `xml:"ObjectLockEnabled,omitempty"`
-	Rule              *ObjectLockRule `xml:"Rule,omitempty"`
+	ObjectLockEnabled string          `xml:"http://s3.amazonaws.com/doc/2006-03-01/ ObjectLockEnabled,omitempty"`
+	Rule              *ObjectLockRule `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Rule,omitempty"`
 }
 
 // ObjectLockRule represents an Object Lock Rule
 type ObjectLockRule struct {
-	XMLName          xml.Name          `xml:"Rule"`
-	DefaultRetention *DefaultRetention `xml:"DefaultRetention,omitempty"`
+	XMLName          xml.Name          `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Rule"`
+	DefaultRetention *DefaultRetention `xml:"http://s3.amazonaws.com/doc/2006-03-01/ DefaultRetention,omitempty"`
 }
 
 // DefaultRetention represents default retention settings
+// Implements custom XML unmarshal to track if Days/Years were present in XML
+
 type DefaultRetention struct {
-	XMLName xml.Name `xml:"DefaultRetention"`
-	Mode    string   `xml:"Mode,omitempty"`
-	Days    int      `xml:"Days,omitempty"`
-	Years   int      `xml:"Years,omitempty"`
+	XMLName  xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ DefaultRetention"`
+	Mode     string   `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Mode,omitempty"`
+	Days     int      `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Days,omitempty"`
+	Years    int      `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Years,omitempty"`
+	DaysSet  bool     `xml:"-"`
+	YearsSet bool     `xml:"-"`
 }
 
-// Custom time unmarshalling for AWS S3 ISO8601 format
-func (or *ObjectRetention) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
-	type Alias ObjectRetention
+func (dr *DefaultRetention) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
+	type Alias DefaultRetention
 	aux := &struct {
 		*Alias
-		RetainUntilDate *string `xml:"RetainUntilDate,omitempty"`
-	}{
-		Alias: (*Alias)(or),
-	}
-
+		Days  *int `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Days,omitempty"`
+		Years *int `xml:"http://s3.amazonaws.com/doc/2006-03-01/ Years,omitempty"`
+	}{Alias: (*Alias)(dr)}
 	if err := d.DecodeElement(aux, &start); err != nil {
+		glog.Errorf("DefaultRetention.UnmarshalXML: decode error: %v", err)
 		return err
 	}
-
-	if aux.RetainUntilDate != nil {
-		t, err := time.Parse(time.RFC3339, *aux.RetainUntilDate)
-		if err != nil {
-			return err
-		}
-		or.RetainUntilDate = &t
+	if aux.Days != nil {
+		dr.Days = *aux.Days
+		dr.DaysSet = true
+		glog.Infof("DefaultRetention.UnmarshalXML: Days present, value=%d", dr.Days)
+	} else {
+		glog.Infof("DefaultRetention.UnmarshalXML: Days not present")
+	}
+	if aux.Years != nil {
+		dr.Years = *aux.Years
+		dr.YearsSet = true
+		glog.Infof("DefaultRetention.UnmarshalXML: Years present, value=%d", dr.Years)
+	} else {
+		glog.Infof("DefaultRetention.UnmarshalXML: Years not present")
 	}
-
 	return nil
 }
 
@@ -221,50 +228,47 @@ func validateObjectLockConfiguration(config *ObjectLockConfiguration) error {
 
 // validateDefaultRetention validates default retention configuration
 func validateDefaultRetention(retention *DefaultRetention) error {
+	glog.Infof("validateDefaultRetention: Mode=%s, Days=%d (set=%v), Years=%d (set=%v)", retention.Mode, retention.Days, retention.DaysSet, retention.Years, retention.YearsSet)
 	// Mode is required
 	if retention.Mode == "" {
 		return ErrDefaultRetentionMissingMode
 	}
-
 	// Mode must be valid
 	if retention.Mode != s3_constants.RetentionModeGovernance && retention.Mode != s3_constants.RetentionModeCompliance {
 		return ErrInvalidDefaultRetentionMode
 	}
-
 	// Check for invalid Years value (negative values are always invalid)
-	if retention.Years < 0 {
+	if retention.YearsSet && retention.Years < 0 {
 		return ErrInvalidRetentionPeriod
 	}
-
 	// Check for invalid Days value (negative values are invalid)
-	if retention.Days < 0 {
+	if retention.DaysSet && retention.Days < 0 {
+		return ErrInvalidRetentionPeriod
+	}
+	// Check for invalid Days value (zero is invalid when explicitly provided)
+	if retention.DaysSet && retention.Days == 0 {
 		return ErrInvalidRetentionPeriod
 	}
-
 	// Check for neither Days nor Years being specified
-	if retention.Days == 0 && retention.Years == 0 {
+	if !retention.DaysSet && !retention.YearsSet {
 		return ErrDefaultRetentionMissingPeriod
 	}
-
 	// Check for both Days and Years being specified
-	if retention.Days > 0 && retention.Years > 0 {
+	if retention.DaysSet && retention.YearsSet && retention.Days > 0 && retention.Years > 0 {
 		return ErrDefaultRetentionBothDaysAndYears
 	}
-
 	// Validate Days if specified
-	if retention.Days > 0 {
+	if retention.DaysSet && retention.Days > 0 {
 		if retention.Days > MaxRetentionDays {
 			return ErrDefaultRetentionDaysOutOfRange
 		}
 	}
-
 	// Validate Years if specified
-	if retention.Years > 0 {
+	if retention.YearsSet && retention.Years > 0 {
 		if retention.Years > MaxRetentionYears {
 			return ErrDefaultRetentionYearsOutOfRange
 		}
 	}
-
 	return nil
 }