From db305142f1f84ffb8c662510eba6ecda6cae3ea7 Mon Sep 17 00:00:00 2001
From: Ben McClelland <ben.mcclelland@versity.com>
Date: Thu, 14 Nov 2024 22:33:08 -0800
Subject: [PATCH] fix: return better error when trying to delete non empty
 directory object

The posix backend will return ENOTEMPTY when trying to delete a
directory that is not empty. This normally would run successfully
on object systems. So we need to create another non-standard error
for this case. We mainly just don't want to return InternalError
for this case.

Fixes #946
---
 backend/posix/posix.go           |  3 +++
 s3err/s3err.go                   |  6 ++++++
 tests/integration/group-tests.go |  2 ++
 tests/integration/tests.go       | 31 +++++++++++++++++++++++++++++++
 4 files changed, 42 insertions(+)

diff --git a/backend/posix/posix.go b/backend/posix/posix.go
index bcee5ecc..b2e86096 100644
--- a/backend/posix/posix.go
+++ b/backend/posix/posix.go
@@ -2646,6 +2646,9 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
 	if errors.Is(err, fs.ErrNotExist) {
 		return nil, s3err.GetAPIError(s3err.ErrNoSuchKey)
 	}
+	if errors.Is(err, syscall.ENOTEMPTY) {
+		return nil, s3err.GetAPIError(s3err.ErrDirectoryNotEmpty)
+	}
 	if err != nil {
 		return nil, fmt.Errorf("delete object: %w", err)
 	}
diff --git a/s3err/s3err.go b/s3err/s3err.go
index 3d194710..23f1a816 100644
--- a/s3err/s3err.go
+++ b/s3err/s3err.go
@@ -144,6 +144,7 @@ const (
 	ErrExistingObjectIsDirectory
 	ErrObjectParentIsFile
 	ErrDirectoryObjectContainsData
+	ErrDirectoryNotEmpty
 	ErrQuotaExceeded
 	ErrVersioningNotConfigured
 
@@ -593,6 +594,11 @@ var errorCodeResponse = map[ErrorCode]APIError{
 		Description:    "Directory object contains data payload.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
+	ErrDirectoryNotEmpty: {
+		Code:           "ErrDirectoryNotEmpty",
+		Description:    "Directory object not empty.",
+		HTTPStatusCode: http.StatusBadRequest,
+	},
 	ErrQuotaExceeded: {
 		Code:           "QuotaExceeded",
 		Description:    "Your request was denied due to quota exceeded.",
diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go
index 23ff3df3..124ea159 100644
--- a/tests/integration/group-tests.go
+++ b/tests/integration/group-tests.go
@@ -519,6 +519,7 @@ func TestPosix(s *S3Conf) {
 	PutObject_name_too_long(s)
 	HeadObject_name_too_long(s)
 	DeleteObject_name_too_long(s)
+	DeleteObject_directory_not_empty(s)
 	// posix specific versioning tests
 	if !s.versioningEnabled {
 		TestVersioningDisabled(s)
@@ -770,6 +771,7 @@ func GetIntTests() IntTests {
 		"ListObjectVersions_VD_success":                                       ListObjectVersions_VD_success,
 		"DeleteObject_non_existing_object":                                    DeleteObject_non_existing_object,
 		"DeleteObject_directory_object_noslash":                               DeleteObject_directory_object_noslash,
+		"DeleteObject_directory_not_empty":                                    DeleteObject_directory_not_empty,
 		"DeleteObject_name_too_long":                                          DeleteObject_name_too_long,
 		"DeleteObject_non_existing_dir_object":                                DeleteObject_non_existing_dir_object,
 		"DeleteObject_success":                                                DeleteObject_success,
diff --git a/tests/integration/tests.go b/tests/integration/tests.go
index a84f69ba..e9c746c9 100644
--- a/tests/integration/tests.go
+++ b/tests/integration/tests.go
@@ -4749,6 +4749,37 @@ func DeleteObject_directory_object_noslash(s *S3Conf) error {
 	})
 }
 
+func DeleteObject_directory_not_empty(s *S3Conf) error {
+	testName := "DeleteObject_directory_not_empty"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		obj := "dir/my-obj"
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err := s3client.PutObject(ctx, &s3.PutObjectInput{
+			Bucket: &bucket,
+			Key:    &obj,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		obj = "dir/"
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
+			Bucket: &bucket,
+			Key:    &obj,
+		})
+		cancel()
+		// object servers will return no error, but the posix backend returns
+		// a non-standard directory not empty. This test is a posix only test
+		// to validate the specific error response.
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrDirectoryNotEmpty)); err != nil {
+			return err
+		}
+		return nil
+	})
+}
+
 func DeleteObject_non_existing_dir_object(s *S3Conf) error {
 	testName := "DeleteObject_non_existing_dir_object"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {