fix: reject DeleteObjects requests with more than 1000 keys

* fix: reject DeleteObjects requests with more than 1000 keys

A DeleteObjects request may name at most 1000 keys; S3 rejects anything
larger with 400 InvalidRequest. The handler parsed the body and passed
every key through authorization and on to the backend, so a 1001-key
request was processed instead of being refused.

Check the count right after the body is parsed, before authorization and
before anything reaches the backend, so an over-limit batch can't be
applied partially. Exactly 1000 keys still succeed, and an empty delete
list keeps its current behavior (200 with an empty result).

Fixes #2196

* fix: return MalformedXML for an oversized DeleteObjects request

A DeleteObjects request naming more than 1000 keys is answered with
MalformedXML, not InvalidRequest. Update the check and the unit and
integration expectations.

---------

Co-authored-by: Tung Lam <lamphamabtung96@gmail.com>

s3api/controllers/base.go

@@ -54,6 +54,7 @@ const (
 	minPartNumber                = 1
 	maxPartNumber                = 10000
 	maxWebsiteConfigurationBytes = 131072
+	maxDeleteObjects             = 1000
 
 	defaultRegion      = "us-east-1"
 	defaultContentType = "binary/octet-stream"

s3api/controllers/bucket-post.go

@@ -56,6 +56,19 @@ func (c S3ApiController) DeleteObjects(ctx fiber.Ctx) (*Response, error) {
 		}, s3err.GetAPIError(s3err.ErrInvalidRequest)
 	}
 
+	// 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.
+	if len(dObj.Objects) > maxDeleteObjects {
+		debuglogger.Logf("delete objects: %d keys exceeds the limit of %d",
+			len(dObj.Objects), maxDeleteObjects)
+		return &Response{
+			MetaOpts: &MetaOptions{
+				BucketOwner: parsedAcl.Owner,
+			},
+		}, s3err.GetAPIError(s3err.ErrMalformedXML)
+	}
+
 	// checkErrs holds one entry per requested object — nil where it may
 	// proceed to the backend, an AWS-shaped denial otherwise. DeleteObjects
 	// supports partial success, so a denial on one object (policy or object

s3api/controllers/bucket-post_test.go

@@ -19,6 +19,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"encoding/xml"
+	"fmt"
 	"io"
 	"net/http"
 	"strings"
@@ -58,6 +59,24 @@ func TestS3ApiController_DeleteObjects(t *testing.T) {
 	})
 	assert.NoError(t, err)
 
+	// 1000 keys is the S3 limit for a single DeleteObjects request; 1001
+	// must be rejected before any of them reaches the backend. The counts
+	// are spelled out rather than derived from the limit constant so the
+	// test pins the documented boundary.
+	const keyLimit = 1000
+	keyLimitObjs := make([]types.ObjectIdentifier, keyLimit+1)
+	for i := range keyLimitObjs {
+		keyLimitObjs[i] = types.ObjectIdentifier{Key: utils.GetStringPtr(fmt.Sprintf("key-%d", i))}
+	}
+	atLimitBody, err := xml.Marshal(s3response.DeleteObjects{Objects: keyLimitObjs[:keyLimit]})
+	assert.NoError(t, err)
+
+	overLimitBody, err := xml.Marshal(s3response.DeleteObjects{Objects: keyLimitObjs})
+	assert.NoError(t, err)
+
+	emptyBody, err := xml.Marshal(s3response.DeleteObjects{Objects: []types.ObjectIdentifier{}})
+	assert.NoError(t, err)
+
 	lockConfig, err := json.Marshal(auth.BucketLockConfig{Enabled: true})
 	assert.NoError(t, err)
 
@@ -157,6 +176,70 @@ func TestS3ApiController_DeleteObjects(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "empty delete list",
+			input: testInput{
+				locals:       defaultLocals,
+				body:         emptyBody,
+				extraMockErr: s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound),
+			},
+			output: testOutput{
+				response: &Response{
+					Data: s3response.DeleteResult{},
+					MetaOpts: &MetaOptions{
+						BucketOwner: "root",
+						EventName:   s3event.EventObjectRemovedDeleteObjects,
+						ObjectCount: 0,
+					},
+				},
+			},
+		},
+		{
+			name: "exactly at the 1000 key limit",
+			input: testInput{
+				locals:       defaultLocals,
+				body:         atLimitBody,
+				extraMockErr: s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound),
+			},
+			output: testOutput{
+				response: &Response{
+					Data: s3response.DeleteResult{},
+					MetaOpts: &MetaOptions{
+						BucketOwner: "root",
+						EventName:   s3event.EventObjectRemovedDeleteObjects,
+						ObjectCount: 1000,
+					},
+				},
+			},
+			configureMock: func(be *BackendMock) {
+				be.DeleteObjectsFunc = func(contextMoqParam context.Context, deleteObjectsInput *s3.DeleteObjectsInput) (s3response.DeleteResult, error) {
+					assert.Len(t, deleteObjectsInput.Delete.Objects, 1000)
+					return s3response.DeleteResult{}, nil
+				}
+			},
+		},
+		{
+			name: "over the 1000 key limit",
+			input: testInput{
+				locals:       defaultLocals,
+				body:         overLimitBody,
+				extraMockErr: s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound),
+			},
+			output: testOutput{
+				response: &Response{
+					MetaOpts: &MetaOptions{
+						BucketOwner: "root",
+					},
+				},
+				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 over-limit request")
+					return s3response.DeleteResult{}, nil
+				}
+			},
+		},
 		{
 			name: "partial success: one object locked, one succeeds",
 			input: testInput{

tests/integration/DeleteObjects.go

@@ -353,3 +353,41 @@ func DeleteObjects_iam_all_locked(s *S3Conf) error {
 		return nil
 	}, withLock())
 }
+
+// DeleteObjects_key_limit pins the S3 limit of 1000 keys per request: a
+// batch of exactly 1000 succeeds, and 1001 is rejected with MalformedXML
+// before any of the keys is deleted.
+func DeleteObjects_key_limit(s *S3Conf) error {
+	testName := "DeleteObjects_key_limit"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		delObjects := make([]types.ObjectIdentifier, 1001)
+		for i := range delObjects {
+			delObjects[i] = types.ObjectIdentifier{Key: getPtr(fmt.Sprintf("key-%d", i))}
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		out, err := s3client.DeleteObjects(ctx, &s3.DeleteObjectsInput{
+			Bucket: &bucket,
+			Delete: &types.Delete{Objects: delObjects[:1000]},
+		})
+		cancel()
+		if err != nil {
+			return fmt.Errorf("expected a 1000 key delete to succeed: %w", err)
+		}
+		if len(out.Deleted) != 1000 {
+			return fmt.Errorf("expected 1000 deleted objects, instead got %v", len(out.Deleted))
+		}
+
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.DeleteObjects(ctx, &s3.DeleteObjectsInput{
+			Bucket: &bucket,
+			Delete: &types.Delete{Objects: delObjects},
+		})
+		cancel()
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrMalformedXML)); err != nil {
+			return fmt.Errorf("expected 1001 keys to be rejected: %w", err)
+		}
+
+		return nil
+	})
+}

tests/integration/group-tests.go

@@ -369,6 +369,7 @@ func TestDeleteObjects(ts *TestState) {
 	ts.Run(DeleteObjects_empty_input)
 	ts.Run(DeleteObjects_non_existing_objects)
 	ts.Run(DeleteObjects_success)
+	ts.Run(DeleteObjects_key_limit)
 }
 
 func TestCopyObject(ts *TestState) {
@@ -3060,6 +3061,7 @@ func GetIntTests() IntTests {
 		"DeleteObjects_empty_input":                                                        DeleteObjects_empty_input,
 		"DeleteObjects_non_existing_objects":                                               DeleteObjects_non_existing_objects,
 		"DeleteObjects_success":                                                            DeleteObjects_success,
+		"DeleteObjects_key_limit":                                                          DeleteObjects_key_limit,
 		"DeleteObjects_iam_mixed_denials_and_success":                                      DeleteObjects_iam_mixed_denials_and_success,
 		"DeleteObjects_iam_all_access_denied":                                              DeleteObjects_iam_all_access_denied,
 		"DeleteObjects_iam_all_locked":                                                     DeleteObjects_iam_all_locked,