fix many issues

weed/s3api/s3api_object_handlers_delete.go

@@ -49,19 +49,17 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 		auditLog = s3err.GetAccessLog(r, http.StatusNoContent, s3err.ErrNone)
 	}
 
-	// Check object lock permissions before deletion (only for versioned buckets)
-	if versioningEnabled {
-		bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
-		if err := s3a.checkObjectLockPermissions(r, bucket, object, versionId, bypassGovernance); err != nil {
-			glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
-			s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
-			return
-		}
-	}
-
 	if versioningEnabled {
 		// Handle versioned delete
 		if versionId != "" {
+			// Check object lock permissions before deleting specific version
+			bypassGovernance := r.Header.Get("x-amz-bypass-governance-retention") == "true"
+			if err := s3a.checkObjectLockPermissions(r, bucket, object, versionId, bypassGovernance); err != nil {
+				glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
+				s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
+				return
+			}
+
 			// Delete specific version
 			err := s3a.deleteSpecificObjectVersion(bucket, object, versionId)
 			if err != nil {
@@ -73,7 +71,7 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 			// Set version ID in response header
 			w.Header().Set("x-amz-version-id", versionId)
 		} else {
-			// Create delete marker (logical delete)
+			// Create delete marker (logical delete) - this is NOT blocked by object lock retention
 			deleteMarkerVersionId, err := s3a.createDeleteMarker(bucket, object)
 			if err != nil {
 				glog.Errorf("Failed to create delete marker: %v", err)

weed/s3api/s3api_object_handlers_put.go

@@ -573,9 +573,9 @@ func mapValidationErrorToS3Error(err error) s3err.ErrorCode {
 		// This matches the test expectations
 		return s3err.ErrInvalidRequest
 	case errors.Is(err, ErrInvalidLegalHoldStatus):
-		// For invalid legal hold status, return InvalidRequest
-		// This matches the test expectations
-		return s3err.ErrInvalidRequest
+		// For invalid legal hold status, return MalformedXML
+		// AWS S3 returns MalformedXML for invalid legal hold status values
+		return s3err.ErrMalformedXML
 	case errors.Is(err, ErrInvalidRetentionDateFormat):
 		// For malformed retention date format, return MalformedDate
 		// This matches the test expectations

weed/s3api/s3api_object_retention_test.go

@@ -498,8 +498,9 @@ func TestValidateObjectLockConfiguration(t *testing.T) {
 				ObjectLockEnabled: "Enabled",
 				Rule: &ObjectLockRule{
 					DefaultRetention: &DefaultRetention{
-						Mode: "GOVERNANCE",
-						Days: 30,
+						Mode:    "GOVERNANCE",
+						Days:    30,
+						DaysSet: true,
 					},
 				},
 			},
@@ -511,8 +512,9 @@ func TestValidateObjectLockConfiguration(t *testing.T) {
 				ObjectLockEnabled: "Enabled",
 				Rule: &ObjectLockRule{
 					DefaultRetention: &DefaultRetention{
-						Mode:  "COMPLIANCE",
-						Years: 1,
+						Mode:     "COMPLIANCE",
+						Years:    1,
+						YearsSet: true,
 					},
 				},
 			},
@@ -545,9 +547,11 @@ func TestValidateObjectLockConfiguration(t *testing.T) {
 				ObjectLockEnabled: "Enabled",
 				Rule: &ObjectLockRule{
 					DefaultRetention: &DefaultRetention{
-						Mode:  "GOVERNANCE",
-						Days:  30,
-						Years: 1,
+						Mode:     "GOVERNANCE",
+						Days:     30,
+						Years:    1,
+						DaysSet:  true,
+						YearsSet: true,
 					},
 				},
 			},
@@ -573,8 +577,9 @@ func TestValidateObjectLockConfiguration(t *testing.T) {
 				ObjectLockEnabled: "Enabled",
 				Rule: &ObjectLockRule{
 					DefaultRetention: &DefaultRetention{
-						Mode: "INVALID_MODE",
-						Days: 30,
+						Mode:    "INVALID_MODE",
+						Days:    30,
+						DaysSet: true,
 					},
 				},
 			},
@@ -587,8 +592,9 @@ func TestValidateObjectLockConfiguration(t *testing.T) {
 				ObjectLockEnabled: "Enabled",
 				Rule: &ObjectLockRule{
 					DefaultRetention: &DefaultRetention{
-						Mode: "GOVERNANCE",
-						Days: 50000,
+						Mode:    "GOVERNANCE",
+						Days:    50000,
+						DaysSet: true,
 					},
 				},
 			},
@@ -601,8 +607,9 @@ func TestValidateObjectLockConfiguration(t *testing.T) {
 				ObjectLockEnabled: "Enabled",
 				Rule: &ObjectLockRule{
 					DefaultRetention: &DefaultRetention{
-						Mode:  "GOVERNANCE",
-						Years: 200,
+						Mode:     "GOVERNANCE",
+						Years:    200,
+						YearsSet: true,
 					},
 				},
 			},
@@ -651,23 +658,26 @@ func TestValidateDefaultRetention(t *testing.T) {
 		{
 			name: "Valid retention with days",
 			retention: &DefaultRetention{
-				Mode: "GOVERNANCE",
-				Days: 30,
+				Mode:    "GOVERNANCE",
+				Days:    30,
+				DaysSet: true,
 			},
 			expectError: false,
 		},
 		{
 			name: "Valid retention with years",
 			retention: &DefaultRetention{
-				Mode:  "COMPLIANCE",
-				Years: 1,
+				Mode:     "COMPLIANCE",
+				Years:    1,
+				YearsSet: true,
 			},
 			expectError: false,
 		},
 		{
 			name: "Missing mode",
 			retention: &DefaultRetention{
-				Days: 30,
+				Days:    30,
+				DaysSet: true,
 			},
 			expectError: true,
 			errorMsg:    "default retention must specify Mode",
@@ -675,8 +685,9 @@ func TestValidateDefaultRetention(t *testing.T) {
 		{
 			name: "Invalid mode",
 			retention: &DefaultRetention{
-				Mode: "INVALID",
-				Days: 30,
+				Mode:    "INVALID",
+				Days:    30,
+				DaysSet: true,
 			},
 			expectError: true,
 			errorMsg:    "invalid default retention mode",
@@ -684,9 +695,11 @@ func TestValidateDefaultRetention(t *testing.T) {
 		{
 			name: "Both days and years specified",
 			retention: &DefaultRetention{
-				Mode:  "GOVERNANCE",
-				Days:  30,
-				Years: 1,
+				Mode:     "GOVERNANCE",
+				Days:     30,
+				Years:    1,
+				DaysSet:  true,
+				YearsSet: true,
 			},
 			expectError: true,
 			errorMsg:    "default retention cannot specify both Days and Years",