From 7203c78e4d3046a9a2006cf521a4921be07d193c Mon Sep 17 00:00:00 2001
From: chrislu <chris.lu@gmail.com>
Date: Fri, 18 Jul 2025 15:58:07 -0700
Subject: [PATCH] constants and fixes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

✅ Return InvalidRetentionPeriod for invalid retention values (0 days, negative years)
✅ Return ObjectLockConfigurationNotFoundError when object lock configuration doesn't exist
✅ Handle all object lock validation errors consistently with proper error codes
---
 weed/s3api/s3api_bucket_config.go                      | 2 +-
 weed/s3api/s3api_bucket_handlers.go                    | 4 ++--
 weed/s3api/s3api_bucket_handlers_object_lock_config.go | 2 +-
 weed/s3api/s3api_object_handlers_put.go                | 6 +++---
 weed/s3api/s3api_object_handlers_retention.go          | 2 +-
 weed/s3api/s3err/s3api_errors.go                       | 6 ++++++
 6 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/weed/s3api/s3api_bucket_config.go b/weed/s3api/s3api_bucket_config.go
index 725ee3596..5bfae7666 100644
--- a/weed/s3api/s3api_bucket_config.go
+++ b/weed/s3api/s3api_bucket_config.go
@@ -225,7 +225,7 @@ func (s3a *S3ApiServer) getBucketVersioningStatus(bucket string) (string, s3err.
 	}
 
 	if config.Versioning == "" {
-		return "Suspended", s3err.ErrNone
+		return s3_constants.VersioningSuspended, s3err.ErrNone
 	}
 
 	return config.Versioning, s3err.ErrNone
diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go
index 2ecac4d11..591aaafb3 100644
--- a/weed/s3api/s3api_bucket_handlers.go
+++ b/weed/s3api/s3api_bucket_handlers.go
@@ -753,13 +753,13 @@ func (s3a *S3ApiServer) PutBucketVersioningHandler(w http.ResponseWriter, r *htt
 	}
 
 	status := *versioningConfig.Status
-	if status != "Enabled" && status != "Suspended" {
+	if status != s3_constants.VersioningEnabled && status != s3_constants.VersioningSuspended {
 		s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
 		return
 	}
 
 	// Check if trying to suspend versioning on a bucket with object lock enabled
-	if status == "Suspended" {
+	if status == s3_constants.VersioningSuspended {
 		// Get bucket configuration to check for object lock
 		bucketConfig, errCode := s3a.getBucketConfig(bucket)
 		if errCode == s3err.ErrNone && bucketConfig.ObjectLockConfig != nil {
diff --git a/weed/s3api/s3api_bucket_handlers_object_lock_config.go b/weed/s3api/s3api_bucket_handlers_object_lock_config.go
index a0f9ba938..2c4be3b33 100644
--- a/weed/s3api/s3api_bucket_handlers_object_lock_config.go
+++ b/weed/s3api/s3api_bucket_handlers_object_lock_config.go
@@ -113,7 +113,7 @@ func (s3a *S3ApiServer) GetObjectLockConfigurationHandler(w http.ResponseWriter,
 
 	// If no Object Lock configuration found, return error
 	if len(configXML) == 0 {
-		s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchObjectLockConfiguration)
+		s3err.WriteErrorResponse(w, r, s3err.ErrObjectLockConfigurationNotFoundError)
 		return
 	}
 
diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go
index 446b78fde..71f2bc2d8 100644
--- a/weed/s3api/s3api_object_handlers_put.go
+++ b/weed/s3api/s3api_object_handlers_put.go
@@ -566,9 +566,9 @@ func mapValidationErrorToS3Error(err error) s3err.ErrorCode {
 	case errors.Is(err, ErrInvalidRetentionDateFormat):
 		return s3err.ErrMalformedXML
 	case errors.Is(err, ErrInvalidRetentionPeriod):
-		// For invalid retention periods (days/years), return MalformedXML
-		// This includes cases where both Days and Years are specified
-		return s3err.ErrMalformedXML
+		// For invalid retention periods (0 days, negative years, etc.), return InvalidRetentionPeriod
+		// This includes cases where retention values are out of valid ranges
+		return s3err.ErrInvalidRetentionPeriod
 	case errors.Is(err, ErrInvalidRetentionMode):
 		// For invalid retention modes, return MalformedXML
 		// This includes cases where ObjectLockEnabled is 'Disabled'
diff --git a/weed/s3api/s3api_object_handlers_retention.go b/weed/s3api/s3api_object_handlers_retention.go
index 687a95955..0414ebbad 100644
--- a/weed/s3api/s3api_object_handlers_retention.go
+++ b/weed/s3api/s3api_object_handlers_retention.go
@@ -102,7 +102,7 @@ func (s3a *S3ApiServer) GetObjectRetentionHandler(w http.ResponseWriter, r *http
 		}
 
 		if errors.Is(err, ErrNoRetentionConfiguration) {
-			s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchObjectLockConfiguration)
+			s3err.WriteErrorResponse(w, r, s3err.ErrObjectLockConfigurationNotFoundError)
 			return
 		}
 
diff --git a/weed/s3api/s3err/s3api_errors.go b/weed/s3api/s3err/s3api_errors.go
index 12608ac21..613b27b95 100644
--- a/weed/s3api/s3err/s3api_errors.go
+++ b/weed/s3api/s3err/s3api_errors.go
@@ -114,6 +114,7 @@ const (
 	ErrNoSuchObjectLockConfiguration
 	ErrNoSuchObjectLegalHold
 	ErrInvalidRetentionPeriod
+	ErrObjectLockConfigurationNotFoundError
 )
 
 // Error message constants for checksum validation
@@ -459,6 +460,11 @@ var errorCodeResponse = map[ErrorCode]APIError{
 		Description:    "The bucket ownership controls were not found",
 		HTTPStatusCode: http.StatusNotFound,
 	},
+	ErrObjectLockConfigurationNotFoundError: {
+		Code:           "ObjectLockConfigurationNotFoundError",
+		Description:    "Object Lock configuration does not exist for this bucket",
+		HTTPStatusCode: http.StatusNotFound,
+	},
 }
 
 // GetAPIError provides API Error for input API error code.