From 62d77006d289405d70e5c80d21d72212fea26097 Mon Sep 17 00:00:00 2001 From: DualFroz Date: Fri, 2 Oct 2026 16:02:57 +0200 Subject: [PATCH] fix: return MalformedXML for a DeleteObjects request without objects A DeleteObjects request whose Delete element names no objects was answered with 200 and an empty DeleteResult. S3 rejects it with MalformedXML, as the schema requires at least one Object. Reject the empty list right after the body is parsed, next to the 1000 key limit check. Update the unit and integration tests that expected the empty result and enable the REST test that covers it. Fixes #2109 --- s3api/controllers/bucket-post.go | 11 +++++++++++ s3api/controllers/bucket-post_test.go | 10 +++++++--- tests/integration/DeleteObjects.go | 12 ++---------- tests/test_rest_delete_objects.sh | 3 --- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/s3api/controllers/bucket-post.go b/s3api/controllers/bucket-post.go index b9a082f..b7545b9 100644 --- a/s3api/controllers/bucket-post.go +++ b/s3api/controllers/bucket-post.go @@ -57,6 +57,17 @@ func (c S3ApiController) DeleteObjects(ctx fiber.Ctx) (*Response, error) { }, s3err.GetAPIError(s3err.ErrInvalidRequest) } + // S3 requires at least one object in a DeleteObjects request and + // rejects an empty delete list as malformed. + if len(dObj.Objects) == 0 { + debuglogger.Logf("delete objects: no keys in the request") + return &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: parsedAcl.Owner, + }, + }, s3err.GetAPIError(s3err.ErrMalformedXML) + } + // S3 caps a single DeleteObjects request at 1000 keys. Reject an // over-limit request before authorization and before anything reaches // the backend, so a too-large batch can't be applied partially. diff --git a/s3api/controllers/bucket-post_test.go b/s3api/controllers/bucket-post_test.go index e7c1dc0..eb63a10 100644 --- a/s3api/controllers/bucket-post_test.go +++ b/s3api/controllers/bucket-post_test.go @@ -241,13 +241,17 @@ func TestS3ApiController_DeleteObjects(t *testing.T) { }, output: testOutput{ response: &Response{ - Data: s3response.DeleteResult{}, MetaOpts: &MetaOptions{ BucketOwner: "root", - EventName: s3event.EventObjectRemovedDeleteObjects, - ObjectCount: 0, }, }, + err: s3err.GetAPIError(s3err.ErrMalformedXML), + }, + configureMock: func(be *BackendMock) { + be.DeleteObjectsFunc = func(contextMoqParam context.Context, deleteObjectsInput *s3.DeleteObjectsInput) (s3response.DeleteResult, error) { + t.Error("backend DeleteObjects called for an empty delete list") + return s3response.DeleteResult{}, nil + } }, }, { diff --git a/tests/integration/DeleteObjects.go b/tests/integration/DeleteObjects.go index bf97149..7162631 100644 --- a/tests/integration/DeleteObjects.go +++ b/tests/integration/DeleteObjects.go @@ -49,25 +49,17 @@ func DeleteObjects_empty_input(s *S3Conf) error { } ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - out, err := s3client.DeleteObjects(ctx, &s3.DeleteObjectsInput{ + _, err = s3client.DeleteObjects(ctx, &s3.DeleteObjectsInput{ Bucket: &bucket, Delete: &types.Delete{ Objects: []types.ObjectIdentifier{}, }, }) cancel() - if err != nil { + if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrMalformedXML)); err != nil { return err } - if len(out.Deleted) != 0 { - return fmt.Errorf("expected deleted object count 0, instead got %v", - len(out.Deleted)) - } - if len(out.Errors) != 0 { - return fmt.Errorf("expected 0 errors, instead got %v", len(out.Errors)) - } - ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) res, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{ Bucket: &bucket, diff --git a/tests/test_rest_delete_objects.sh b/tests/test_rest_delete_objects.sh index 29c4890..9bb7ecb 100644 --- a/tests/test_rest_delete_objects.sh +++ b/tests/test_rest_delete_objects.sh @@ -36,9 +36,6 @@ source ./tests/setup.sh # tags: curl, DeleteObjects, malformed-message @test "REST - DeleteObjects - no objects added to payload" { - if [ "$DIRECT" != "true" ]; then - skip "https://github.com/versity/versitygw/issues/2109" - fi run setup_bucket_and_file_v3 "$BUCKET_ONE_NAME" assert_success read -r bucket_name file_name <<< "$output"