From 1db149ecc24f5d8771436975a315852637a29d0e Mon Sep 17 00:00:00 2001 From: chrislu Date: Fri, 18 Jul 2025 01:49:44 -0700 Subject: [PATCH] address comments --- weed/s3api/object_lock_utils.go | 32 ++----------------- weed/s3api/s3api_bucket_config.go | 2 +- weed/s3api/s3api_bucket_handlers.go | 2 +- ...3api_bucket_handlers_object_lock_config.go | 4 +++ weed/s3api/s3api_object_handlers_put.go | 3 +- 5 files changed, 10 insertions(+), 33 deletions(-) diff --git a/weed/s3api/object_lock_utils.go b/weed/s3api/object_lock_utils.go index 771004e24..e16ee4dc9 100644 --- a/weed/s3api/object_lock_utils.go +++ b/weed/s3api/object_lock_utils.go @@ -4,7 +4,6 @@ import ( "encoding/xml" "fmt" "strconv" - "strings" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" @@ -218,41 +217,16 @@ func ValidateObjectLockParameters(enabled bool, mode string, duration int32) err } if mode != s3_constants.RetentionModeGovernance && mode != s3_constants.RetentionModeCompliance { - return fmt.Errorf("invalid object lock mode: %s, must be GOVERNANCE or COMPLIANCE", mode) + return fmt.Errorf("invalid object lock mode: %s, must be %s or %s", mode, s3_constants.RetentionModeGovernance, s3_constants.RetentionModeCompliance) } if duration <= 0 { - return fmt.Errorf("object lock duration must be greater than 0 days") + return fmt.Errorf("object lock duration must be greater than 0 days, got: %d", duration) } if duration > MaxRetentionDays { - return fmt.Errorf("object lock duration exceeds maximum allowed days: %d", MaxRetentionDays) + return fmt.Errorf("object lock duration exceeds maximum allowed days: %d, got: %d", MaxRetentionDays, duration) } return nil } - -// SimpleXMLParseObjectLockMode extracts mode from XML string using simple string parsing -// This is used as a fallback when full XML parsing is not needed -func SimpleXMLParseObjectLockMode(xmlStr string) string { - if strings.Contains(xmlStr, "GOVERNANCE") { - return "GOVERNANCE" - } else if strings.Contains(xmlStr, "COMPLIANCE") { - return "COMPLIANCE" - } - return "" -} - -// SimpleXMLParseObjectLockDays extracts days from XML string using simple string parsing -// This is used as a fallback when full XML parsing is not needed -func SimpleXMLParseObjectLockDays(xmlStr string) int32 { - if daysStart := strings.Index(xmlStr, ""); daysStart != -1 { - daysStart += 6 // length of "" - if daysEnd := strings.Index(xmlStr[daysStart:], ""); daysEnd != -1 { - if duration, err := strconv.ParseInt(xmlStr[daysStart:daysStart+daysEnd], 10, 32); err == nil { - return int32(duration) - } - } - } - return 0 -} diff --git a/weed/s3api/s3api_bucket_config.go b/weed/s3api/s3api_bucket_config.go index 725ee3596..4f629023d 100644 --- a/weed/s3api/s3api_bucket_config.go +++ b/weed/s3api/s3api_bucket_config.go @@ -211,7 +211,7 @@ func (s3a *S3ApiServer) isVersioningEnabled(bucket string) (bool, error) { if errCode == s3err.ErrNoSuchBucket { return false, filer_pb.ErrNotFound } - return false, fmt.Errorf("failed to get bucket config: %v", errCode) + return false, fmt.Errorf("failed to get bucket config: %s", errCode) } return config.Versioning == "Enabled", nil diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index e30f172a7..6e3688f44 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -159,7 +159,7 @@ func (s3a *S3ApiServer) PutBucketHandler(w http.ResponseWriter, r *http.Request) }) if errCode != s3err.ErrNone { - glog.Errorf("PutBucketHandler: failed to enable Object Lock for bucket %s: %v", bucket, errCode) + glog.Errorf("PutBucketHandler: failed to enable Object Lock for bucket %s: %s", bucket, errCode) s3err.WriteErrorResponse(w, r, errCode) return } diff --git a/weed/s3api/s3api_bucket_handlers_object_lock_config.go b/weed/s3api/s3api_bucket_handlers_object_lock_config.go index 712a6a1bf..494f203a4 100644 --- a/weed/s3api/s3api_bucket_handlers_object_lock_config.go +++ b/weed/s3api/s3api_bucket_handlers_object_lock_config.go @@ -86,6 +86,10 @@ func (s3a *S3ApiServer) GetObjectLockConfigurationHandler(w http.ResponseWriter, // Write XML response w.Header().Set("Content-Type", "application/xml") w.WriteHeader(http.StatusOK) + if _, err := w.Write([]byte(xml.Header)); err != nil { + glog.Errorf("GetObjectLockConfigurationHandler: failed to write XML header: %v", err) + return + } if _, err := w.Write(marshaledXML); err != nil { glog.Errorf("GetObjectLockConfigurationHandler: failed to write config XML: %v", err) return diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index a0f633b3f..e4c966562 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -12,7 +12,6 @@ import ( "time" "github.com/pquerna/cachecontrol/cacheobject" - "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" @@ -437,7 +436,7 @@ func (s3a *S3ApiServer) applyBucketDefaultRetention(bucket string, entry *filer_ // Get bucket configuration (getBucketConfig handles caching internally) bucketConfig, errCode := s3a.getBucketConfig(bucket) if errCode != s3err.ErrNone { - return fmt.Errorf("failed to get bucket config: %v", errCode) + return fmt.Errorf("failed to get bucket config: %s", errCode) } // Check if bucket has cached Object Lock configuration