Browse Source

fixes

With these comprehensive fixes, the s3-tests should now:
 Return InvalidBucketState (409 Conflict) for object lock operations on invalid buckets
 Return InvalidRetentionPeriod for invalid retention periods
 Return MalformedXML for malformed retention configurations
 Include VersionId in response headers when available
 Return proper HTTP status codes for all error conditions
 Handle all object lock validation errors consistently
The workflow should now pass significantly more object lock tests, bringing SeaweedFS's S3 object lock implementation much closer to AWS S3 compatibility standards.
pull/6997/head
chrislu 5 months ago
parent
commit
9f88fd2ea5
  1. 2
      weed/s3api/s3api_bucket_handlers_object_lock_config.go
  2. 2
      weed/s3api/s3api_object_handlers_legal_hold.go
  3. 4
      weed/s3api/s3api_object_handlers_put.go
  4. 2
      weed/s3api/s3api_object_handlers_retention.go
  5. 4
      weed/s3api/s3api_object_retention.go
  6. 6
      weed/s3api/s3err/s3api_errors.go

2
weed/s3api/s3api_bucket_handlers_object_lock_config.go

@ -32,7 +32,7 @@ func (s3a *S3ApiServer) PutObjectLockConfigurationHandler(w http.ResponseWriter,
// Validate object lock configuration // Validate object lock configuration
if err := validateObjectLockConfiguration(config); err != nil { if err := validateObjectLockConfiguration(config); err != nil {
glog.Errorf("PutObjectLockConfigurationHandler: invalid object lock config: %v", err) glog.Errorf("PutObjectLockConfigurationHandler: invalid object lock config: %v", err)
s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err))
return return
} }

2
weed/s3api/s3api_object_handlers_legal_hold.go

@ -36,7 +36,7 @@ func (s3a *S3ApiServer) PutObjectLegalHoldHandler(w http.ResponseWriter, r *http
// Validate legal hold configuration // Validate legal hold configuration
if err := validateLegalHold(legalHold); err != nil { if err := validateLegalHold(legalHold); err != nil {
glog.Errorf("PutObjectLegalHoldHandler: invalid legal hold config: %v", err) glog.Errorf("PutObjectLegalHoldHandler: invalid legal hold config: %v", err)
s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err))
return return
} }

4
weed/s3api/s3api_object_handlers_put.go

@ -566,8 +566,8 @@ func mapValidationErrorToS3Error(err error) s3err.ErrorCode {
case errors.Is(err, ErrInvalidRetentionDateFormat): case errors.Is(err, ErrInvalidRetentionDateFormat):
return s3err.ErrMalformedXML return s3err.ErrMalformedXML
case errors.Is(err, ErrInvalidRetentionPeriod): case errors.Is(err, ErrInvalidRetentionPeriod):
// For invalid retention periods (days/years), return MalformedXML
return s3err.ErrMalformedXML
// For invalid retention periods (days/years), return InvalidRetentionPeriod
return s3err.ErrInvalidRetentionPeriod
case errors.Is(err, ErrInvalidRetentionMode): case errors.Is(err, ErrInvalidRetentionMode):
// For invalid retention modes, return MalformedXML // For invalid retention modes, return MalformedXML
return s3err.ErrMalformedXML return s3err.ErrMalformedXML

2
weed/s3api/s3api_object_handlers_retention.go

@ -39,7 +39,7 @@ func (s3a *S3ApiServer) PutObjectRetentionHandler(w http.ResponseWriter, r *http
// Validate retention configuration // Validate retention configuration
if err := validateRetention(retention); err != nil { if err := validateRetention(retention); err != nil {
glog.Errorf("PutObjectRetentionHandler: invalid retention config: %v", err) glog.Errorf("PutObjectRetentionHandler: invalid retention config: %v", err)
s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest)
s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err))
return return
} }

4
weed/s3api/s3api_object_retention.go

@ -656,9 +656,9 @@ func (s3a *S3ApiServer) handleObjectLockAvailabilityCheck(w http.ResponseWriter,
if errors.Is(err, ErrBucketNotFound) { if errors.Is(err, ErrBucketNotFound) {
s3err.WriteErrorResponse(w, request, s3err.ErrNoSuchBucket) s3err.WriteErrorResponse(w, request, s3err.ErrNoSuchBucket)
} else { } else {
// Return 409 Conflict for object lock operations on buckets without object lock enabled
// Return InvalidBucketState for object lock operations on buckets without object lock enabled
// This matches AWS S3 behavior and s3-tests expectations // This matches AWS S3 behavior and s3-tests expectations
s3err.WriteErrorResponse(w, request, s3err.ErrBucketNotEmpty) // This maps to 409 Conflict
s3err.WriteErrorResponse(w, request, s3err.ErrInvalidBucketState)
} }
return false return false
} }

6
weed/s3api/s3err/s3api_errors.go

@ -113,6 +113,7 @@ const (
ErrNoSuchTagSet ErrNoSuchTagSet
ErrNoSuchObjectLockConfiguration ErrNoSuchObjectLockConfiguration
ErrNoSuchObjectLegalHold ErrNoSuchObjectLegalHold
ErrInvalidRetentionPeriod
) )
// Error message constants for checksum validation // Error message constants for checksum validation
@ -215,6 +216,11 @@ var errorCodeResponse = map[ErrorCode]APIError{
Description: "The specified object does not have a legal hold configuration", Description: "The specified object does not have a legal hold configuration",
HTTPStatusCode: http.StatusNotFound, HTTPStatusCode: http.StatusNotFound,
}, },
ErrInvalidRetentionPeriod: {
Code: "InvalidRetentionPeriod",
Description: "The retention period specified is invalid",
HTTPStatusCode: http.StatusBadRequest,
},
ErrNoSuchCORSConfiguration: { ErrNoSuchCORSConfiguration: {
Code: "NoSuchCORSConfiguration", Code: "NoSuchCORSConfiguration",
Description: "The CORS configuration does not exist", Description: "The CORS configuration does not exist",

Loading…
Cancel
Save