From 1bc503530a4e7c566242e0b9067f18daec2a349c Mon Sep 17 00:00:00 2001 From: chrislu Date: Fri, 18 Jul 2025 15:52:28 -0700 Subject: [PATCH] fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With these final fixes, the s3-tests should now: ✅ Return MalformedXML for ObjectLockEnabled: 'Disabled' ✅ Return MalformedXML when both Days and Years are specified in retention configuration ✅ Return InvalidBucketState (409 Conflict) when trying to suspend versioning on buckets with object lock enabled ✅ Handle all object lock validation errors consistently with proper error codes --- weed/s3api/s3api_bucket_handlers.go | 11 +++++++++++ weed/s3api/s3api_object_handlers_put.go | 8 +++++--- weed/s3api/s3api_object_retention.go | 7 ++++--- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index e30f172a7..2ecac4d11 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -758,6 +758,17 @@ func (s3a *S3ApiServer) PutBucketVersioningHandler(w http.ResponseWriter, r *htt return } + // Check if trying to suspend versioning on a bucket with object lock enabled + if status == "Suspended" { + // Get bucket configuration to check for object lock + bucketConfig, errCode := s3a.getBucketConfig(bucket) + if errCode == s3err.ErrNone && bucketConfig.ObjectLockConfig != nil { + // Object lock is enabled, cannot suspend versioning + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidBucketState) + return + } + } + // Update bucket versioning configuration using new bucket config system if errCode := s3a.setBucketVersioningStatus(bucket, status); errCode != s3err.ErrNone { glog.Errorf("PutBucketVersioningHandler save config: %d", errCode) diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index d2c774aee..446b78fde 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -559,17 +559,19 @@ func mapValidationErrorToS3Error(err error) s3err.ErrorCode { // This matches AWS S3 behavior and s3-tests expectations return s3err.ErrInvalidBucketState case errors.Is(err, ErrInvalidObjectLockMode): - return s3err.ErrInvalidRequest + return s3err.ErrMalformedXML case errors.Is(err, ErrInvalidLegalHoldStatus): // For malformed legal hold status, return MalformedXML as expected by s3-tests return s3err.ErrMalformedXML case errors.Is(err, ErrInvalidRetentionDateFormat): return s3err.ErrMalformedXML case errors.Is(err, ErrInvalidRetentionPeriod): - // For invalid retention periods (days/years), return InvalidRetentionPeriod - return s3err.ErrInvalidRetentionPeriod + // For invalid retention periods (days/years), return MalformedXML + // This includes cases where both Days and Years are specified + return s3err.ErrMalformedXML case errors.Is(err, ErrInvalidRetentionMode): // For invalid retention modes, return MalformedXML + // This includes cases where ObjectLockEnabled is 'Disabled' return s3err.ErrMalformedXML default: return s3err.ErrInvalidRequest diff --git a/weed/s3api/s3api_object_retention.go b/weed/s3api/s3api_object_retention.go index 5d4f846ab..cffb12bca 100644 --- a/weed/s3api/s3api_object_retention.go +++ b/weed/s3api/s3api_object_retention.go @@ -188,18 +188,19 @@ func validateLegalHold(legalHold *ObjectLegalHold) error { func validateObjectLockConfiguration(config *ObjectLockConfiguration) error { // ObjectLockEnabled is required for bucket-level configuration if config.ObjectLockEnabled == "" { - return fmt.Errorf("object lock configuration must specify ObjectLockEnabled") + return ErrInvalidRetentionMode } // Validate ObjectLockEnabled value if config.ObjectLockEnabled != s3_constants.ObjectLockEnabled { - return fmt.Errorf("invalid object lock enabled value: %s", config.ObjectLockEnabled) + // ObjectLockEnabled can only be 'Enabled', any other value (including 'Disabled') is malformed XML + return ErrInvalidRetentionMode } // Validate Rule if present if config.Rule != nil { if config.Rule.DefaultRetention == nil { - return fmt.Errorf("rule configuration must specify DefaultRetention") + return ErrInvalidRetentionPeriod } return validateDefaultRetention(config.Rule.DefaultRetention) }