From 3dc654eb110051eda7458b8ca437a4279f09ce02 Mon Sep 17 00:00:00 2001
From: jonaustin09 <jonaustin460@gmail.com>
Date: Thu, 26 Sep 2024 16:41:35 -0400
Subject: [PATCH] fix: Modified DeleteObject error handling to return a
 successful response when versionId is not specified, and to return
 InvalidVersionId error when it is specified, in cases where versioning is
 enabled.

---
 backend/posix/posix.go           | 11 ++++++-----
 tests/integration/group-tests.go |  2 ++
 tests/integration/tests.go       | 30 ++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/backend/posix/posix.go b/backend/posix/posix.go
index e1400628..ddef1d1d 100644
--- a/backend/posix/posix.go
+++ b/backend/posix/posix.go
@@ -2158,6 +2158,10 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
 		if getString(input.VersionId) == "" {
 			// if the versionId is not specified, make the current version a delete marker
 			fi, err := os.Stat(objpath)
+			if errors.Is(err, fs.ErrNotExist) {
+				// AWS returns success if the object does not exist
+				return &s3.DeleteObjectOutput{}, nil
+			}
 			if errors.Is(err, syscall.ENAMETOOLONG) {
 				return nil, s3err.GetAPIError(s3err.ErrKeyTooLong)
 			}
@@ -2193,7 +2197,7 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
 			versionPath := p.genObjVersionPath(bucket, object)
 
 			vId, err := p.meta.RetrieveAttribute(bucket, object, versionIdKey)
-			if err != nil && !errors.Is(err, meta.ErrNoSuchKey) {
+			if err != nil && !errors.Is(err, meta.ErrNoSuchKey) && !errors.Is(err, fs.ErrNotExist) {
 				return nil, fmt.Errorf("get obj versionId: %w", err)
 			}
 
@@ -2292,10 +2296,7 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
 				return nil, s3err.GetAPIError(s3err.ErrKeyTooLong)
 			}
 			if errors.Is(err, fs.ErrNotExist) {
-				return &s3.DeleteObjectOutput{
-					DeleteMarker: &isDelMarker,
-					VersionId:    input.VersionId,
-				}, nil
+				return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId)
 			}
 			if err != nil {
 				return nil, fmt.Errorf("delete object: %w", err)
diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go
index 7a6cb301..275632bc 100644
--- a/tests/integration/group-tests.go
+++ b/tests/integration/group-tests.go
@@ -537,6 +537,7 @@ func TestVersioning(s *S3Conf) {
 	Versioning_GetObject_delete_marker(s)
 	// DeleteObject(s) actions
 	Versioning_DeleteObject_delete_object_version(s)
+	Versioning_DeleteObject_non_existing_object(s)
 	Versioning_DeleteObject_delete_a_delete_marker(s)
 	Versioning_DeleteObjects_success(s)
 	Versioning_DeleteObjects_delete_deleteMarkers(s)
@@ -884,6 +885,7 @@ func GetIntTests() IntTests {
 		"Versioning_GetObject_success":                                        Versioning_GetObject_success,
 		"Versioning_GetObject_delete_marker":                                  Versioning_GetObject_delete_marker,
 		"Versioning_DeleteObject_delete_object_version":                       Versioning_DeleteObject_delete_object_version,
+		"Versioning_DeleteObject_non_existing_object":                         Versioning_DeleteObject_non_existing_object,
 		"Versioning_DeleteObject_delete_a_delete_marker":                      Versioning_DeleteObject_delete_a_delete_marker,
 		"Versioning_DeleteObjects_success":                                    Versioning_DeleteObjects_success,
 		"Versioning_DeleteObjects_delete_deleteMarkers":                       Versioning_DeleteObjects_delete_deleteMarkers,
diff --git a/tests/integration/tests.go b/tests/integration/tests.go
index 45776800..c118c699 100644
--- a/tests/integration/tests.go
+++ b/tests/integration/tests.go
@@ -10927,6 +10927,36 @@ func Versioning_DeleteObject_delete_object_version(s *S3Conf) error {
 	}, withVersioning())
 }
 
+func Versioning_DeleteObject_non_existing_object(s *S3Conf) error {
+	testName := "Versioning_DeleteObject_non_existing_object"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		obj := "my-obj"
+
+		ctx, canel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
+			Bucket: &bucket,
+			Key:    &obj,
+		})
+		canel()
+		if err != nil {
+			return err
+		}
+
+		ctx, canel = context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
+			Bucket:    &bucket,
+			Key:       &obj,
+			VersionId: getPtr("non_existing_version_id"),
+		})
+		canel()
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidVersionId)); err != nil {
+			return err
+		}
+
+		return nil
+	}, withVersioning())
+}
+
 func Versioning_DeleteObject_delete_a_delete_marker(s *S3Conf) error {
 	testName := "Versioning_DeleteObject_delete_a_delete_marker"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {