Fix DeleteMessageBatch form parsing

Admiral-Piett · Nov 8, 2024 · 885f4dd · 885f4dd
1 parent 913294f
commit 885f4dd
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 19 deletions.
diff --git a/app/gosqs/delete_message_batch.go b/app/gosqs/delete_message_batch.go
@@ -12,10 +12,8 @@ import (
 )
 
 func DeleteMessageBatchV1(req *http.Request) (int, interfaces.AbstractResponseBody) {
-
 	requestBody := models.NewDeleteMessageBatchRequest()
 	ok := utils.REQUEST_TRANSFORMER(requestBody, req, false)
-
 	if !ok {
 		log.Error("Invalid Request - DeleteMessageBatchV1")
 		return utils.CreateErrorResponseV1("InvalidParameterValue", true)
@@ -44,12 +42,12 @@ func DeleteMessageBatchV1(req *http.Request) (int, interfaces.AbstractResponseBo
 		return utils.CreateErrorResponseV1("TooManyEntriesInBatchRequest", true)
 	}
 
-	ids := map[string]struct{}{}
+	ids := map[string]bool{}
 	for _, v := range requestBody.Entries {
-		if _, ok := ids[v.Id]; ok {
+		if _, found := ids[v.Id]; found {
 			return utils.CreateErrorResponseV1("BatchEntryIdsNotDistinct", true)
 		}
-		ids[v.Id] = struct{}{}
+		ids[v.Id] = true
 	}
 
 	models.SyncQueues.Lock()

diff --git a/app/models/requests.go b/app/models/requests.go
@@ -525,15 +525,34 @@ type DeleteMessageBatchRequestEntry struct {
 }
 
 type DeleteMessageBatchRequest struct {
-	Entries  []DeleteMessageBatchRequestEntry `json:"Entries" schema:"Entries"`
+	Entries  []DeleteMessageBatchRequestEntry `json:"Entries"`
 	QueueUrl string                           `json:"QueueUrl" schema:"QueueUrl"`
 }
 
 func NewDeleteMessageBatchRequest() *DeleteMessageBatchRequest {
 	return &DeleteMessageBatchRequest{}
 }
 
-func (r *DeleteMessageBatchRequest) SetAttributesFromForm(values url.Values) {}
+func (r *DeleteMessageBatchRequest) SetAttributesFromForm(values url.Values) {
+	entries := []DeleteMessageBatchRequestEntry{}
+	for i := 1; true; i++ {
+		msgIdKey := fmt.Sprintf("DeleteMessageBatchRequestEntry.%d.Id", i)
+		receiptHandleKey := fmt.Sprintf("DeleteMessageBatchRequestEntry.%d.ReceiptHandle", i)
+
+		msgId := values.Get(msgIdKey)
+		receiptHandle := values.Get(receiptHandleKey)
+		if msgId == "" || receiptHandle == "" {
+			break
+		}
+		entries = append(entries, DeleteMessageBatchRequestEntry{
+			Id:            msgId,
+			ReceiptHandle: receiptHandle,
+		})
+	}
+	if len(entries) > 0 {
+		r.Entries = entries
+	}
+}
 
 // ---- SNS ----
 func NewCreateTopicRequest() *CreateTopicRequest {

diff --git a/app/models/requests_test.go b/app/models/requests_test.go
@@ -566,3 +566,58 @@ func TestSubscribeRequest_SetAttributesFromForm_stops_if_attributes_not_numbered
 	assert.False(t, cqr.Attributes.RawMessageDelivery)
 	assert.Equal(t, FilterPolicy(nil), cqr.Attributes.FilterPolicy)
 }
+
+func Test_DeleteMessageBatchRequest_SetAttributesFromForm_success(t *testing.T) {
+	form := url.Values{}
+	form.Add("DeleteMessageBatchRequestEntry.1.Id", "message-id-1")
+	form.Add("DeleteMessageBatchRequestEntry.1.ReceiptHandle", "receipt-handle-1")
+	form.Add("DeleteMessageBatchRequestEntry.2.Id", "message-id-2")
+	form.Add("DeleteMessageBatchRequestEntry.2.ReceiptHandle", "receipt-handle-2")
+	form.Add("DeleteMessageBatchRequestEntry.3.Id", "message-id-3")
+	form.Add("DeleteMessageBatchRequestEntry.3.ReceiptHandle", "receipt-handle-3")
+
+	dmbr := &DeleteMessageBatchRequest{}
+	dmbr.SetAttributesFromForm(form)
+
+	assert.Len(t, dmbr.Entries, 3)
+	assert.Equal(t, "message-id-1", dmbr.Entries[0].Id)
+	assert.Equal(t, "receipt-handle-1", dmbr.Entries[0].ReceiptHandle)
+	assert.Equal(t, "message-id-2", dmbr.Entries[1].Id)
+	assert.Equal(t, "receipt-handle-2", dmbr.Entries[1].ReceiptHandle)
+	assert.Equal(t, "message-id-3", dmbr.Entries[2].Id)
+	assert.Equal(t, "receipt-handle-3", dmbr.Entries[2].ReceiptHandle)
+}
+
+func Test_DeleteMessageBatchRequest_SetAttributesFromForm_stops_at_non_sequential_keys(t *testing.T) {
+	form := url.Values{}
+	form.Add("DeleteMessageBatchRequestEntry.1.Id", "message-id-1")
+	form.Add("DeleteMessageBatchRequestEntry.1.ReceiptHandle", "receipt-handle-1")
+	form.Add("DeleteMessageBatchRequestEntry.4.Id", "message-id-2")
+	form.Add("DeleteMessageBatchRequestEntry.4.ReceiptHandle", "receipt-handle-2")
+	form.Add("DeleteMessageBatchRequestEntry.3.Id", "message-id-3")
+	form.Add("DeleteMessageBatchRequestEntry.3.ReceiptHandle", "receipt-handle-3")
+
+	dmbr := &DeleteMessageBatchRequest{}
+	dmbr.SetAttributesFromForm(form)
+
+	assert.Len(t, dmbr.Entries, 1)
+	assert.Equal(t, "message-id-1", dmbr.Entries[0].Id)
+	assert.Equal(t, "receipt-handle-1", dmbr.Entries[0].ReceiptHandle)
+}
+
+func Test_DeleteMessageBatchRequest_SetAttributesFromForm_stops_at_invalid_keys(t *testing.T) {
+	form := url.Values{}
+	form.Add("DeleteMessageBatchRequestEntry.1.Id", "message-id-1")
+	form.Add("DeleteMessageBatchRequestEntry.1.ReceiptHandle", "receipt-handle-1")
+	form.Add("INVALID_DeleteMessageBatchRequestEntry.2.Id", "message-id-2")
+	form.Add("DeleteMessageBatchRequestEntry.2.ReceiptHandle", "receipt-handle-2")
+	form.Add("DeleteMessageBatchRequestEntry.3.Id", "message-id-3")
+	form.Add("DeleteMessageBatchRequestEntry.3.ReceiptHandle", "receipt-handle-3")
+
+	dmbr := &DeleteMessageBatchRequest{}
+	dmbr.SetAttributesFromForm(form)
+
+	assert.Len(t, dmbr.Entries, 1)
+	assert.Equal(t, "message-id-1", dmbr.Entries[0].Id)
+	assert.Equal(t, "receipt-handle-1", dmbr.Entries[0].ReceiptHandle)
+}
diff --git a/smoke_tests/sqs_delete_message_batch_test.go b/smoke_tests/sqs_delete_message_batch_test.go
@@ -468,12 +468,12 @@ func Test_DeleteMessageBatchV1_xml_success_not_found_message(t *testing.T) {
 	// delete messages
 	deletedMessages := e.POST("/").
 		WithForm(deleteMessageBatchRequestBodyXML).
-		WithFormField("Entries.0.Id", testId1).
-		WithFormField("Entries.0.ReceiptHandle", *receivedMessageResponse1.Messages[0].ReceiptHandle).
-		WithFormField("Entries.1.Id", testId2).
-		WithFormField("Entries.1.ReceiptHandle", *receivedMessageResponse2.Messages[0].ReceiptHandle).
-		WithFormField("Entries.2.Id", testId3).
-		WithFormField("Entries.2.ReceiptHandle", *receivedMessageResponse1.Messages[1].ReceiptHandle).
+		WithFormField("DeleteMessageBatchRequestEntry.1.Id", testId1).
+		WithFormField("DeleteMessageBatchRequestEntry.1.ReceiptHandle", *receivedMessageResponse1.Messages[0].ReceiptHandle).
+		WithFormField("DeleteMessageBatchRequestEntry.2.Id", testId2).
+		WithFormField("DeleteMessageBatchRequestEntry.2.ReceiptHandle", *receivedMessageResponse2.Messages[0].ReceiptHandle).
+		WithFormField("DeleteMessageBatchRequestEntry.3.Id", testId3).
+		WithFormField("DeleteMessageBatchRequestEntry.3.ReceiptHandle", *receivedMessageResponse1.Messages[1].ReceiptHandle).
 		Expect().
 		Status(http.StatusOK).
 		Body().Raw()
@@ -563,12 +563,12 @@ func Test_DeleteMessageBatchV1_xml_success_all_deletes(t *testing.T) {
 	// delete messages
 	deletedMessages := e.POST("/").
 		WithForm(deleteMessageBatchRequestBodyXML).
-		WithFormField("Entries.0.Id", testId1).
-		WithFormField("Entries.0.ReceiptHandle", *receivedMessageResponse.Messages[0].ReceiptHandle).
-		WithFormField("Entries.1.Id", testId2).
-		WithFormField("Entries.1.ReceiptHandle", *receivedMessageResponse.Messages[1].ReceiptHandle).
-		WithFormField("Entries.2.Id", testId3).
-		WithFormField("Entries.2.ReceiptHandle", *receivedMessageResponse.Messages[2].ReceiptHandle).
+		WithFormField("DeleteMessageBatchRequestEntry.1.Id", testId1).
+		WithFormField("DeleteMessageBatchRequestEntry.1.ReceiptHandle", *receivedMessageResponse.Messages[0].ReceiptHandle).
+		WithFormField("DeleteMessageBatchRequestEntry.2.Id", testId2).
+		WithFormField("DeleteMessageBatchRequestEntry.2.ReceiptHandle", *receivedMessageResponse.Messages[1].ReceiptHandle).
+		WithFormField("DeleteMessageBatchRequestEntry.3.Id", testId3).
+		WithFormField("DeleteMessageBatchRequestEntry.3.ReceiptHandle", *receivedMessageResponse.Messages[2].ReceiptHandle).
 		Expect().
 		Status(http.StatusOK).
 		Body().Raw()