PR Feedback

packages/server/src/api/controllers/row/index.ts

@@ -2,7 +2,14 @@ import { quotas } from "@budibase/pro"
 import * as internal from "./internal"
 import * as external from "./external"
 import { isExternalTable } from "../../../integrations/utils"
-import { Ctx, UserCtx, DeleteRowRequest, Row } from "@budibase/types"
+import {
+  Ctx,
+  UserCtx,
+  DeleteRowRequest,
+  DeleteRow,
+  DeleteRows,
+  Row,
+} from "@budibase/types"
 import * as utils from "./utils"
 import { gridSocket } from "../../../websockets"
 import { addRev } from "../public/utils"
@@ -100,49 +107,83 @@ export async function find(ctx: any) {
   })
 }
 
-export async function destroy(ctx: UserCtx<DeleteRowRequest>) {
-  const appId = ctx.appId
-  const inputs = ctx.request.body
+function isDeleteRows(input: any): input is DeleteRows {
+  return input.rows !== undefined && Array.isArray(input.rows)
+}
 
+function isDeleteRow(input: any): input is DeleteRow {
+  return input._id !== undefined
+}
+
+async function processDeleteRowRequest(ctx: UserCtx<DeleteRowRequest>) {
+  let request = ctx.request.body as DeleteRows
   const tableId = utils.getTableId(ctx)
-  let response, row
 
-  if ("rows" in inputs && Array.isArray(inputs?.rows)) {
-    const targetRows = inputs.rows.map(row => {
-      let processedRow: Row = typeof row == "string" ? { _id: row } : row
-      return !processedRow._rev
-        ? addRev(fixRow(processedRow, ctx.params), tableId)
-        : fixRow(processedRow, ctx.params)
-    })
+  const processedRows = request.rows.map(row => {
+    let processedRow: Row = typeof row == "string" ? { _id: row } : row
+    return !processedRow._rev
+      ? addRev(fixRow(processedRow, ctx.params), tableId)
+      : fixRow(processedRow, ctx.params)
+  })
 
-    const rowDeletes: Row[] = await Promise.all(targetRows)
-    if (rowDeletes) {
-      inputs.rows = rowDeletes
-    }
+  return await Promise.all(processedRows)
+}
 
-    let { rows } = await quotas.addQuery<any>(
-      () => pickApi(tableId).bulkDestroy(ctx),
-      {
-        datasourceId: tableId,
-      }
-    )
-    await quotas.removeRows(rows.length)
-    response = rows
-    for (let row of rows) {
-      ctx.eventEmitter && ctx.eventEmitter.emitRow(`row:delete`, appId, row)
-      gridSocket?.emitRowDeletion(ctx, row._id)
-    }
-  } else {
-    let resp = await quotas.addQuery<any>(() => pickApi(tableId).destroy(ctx), {
+async function deleteRows(ctx: UserCtx<DeleteRowRequest>) {
+  const tableId = utils.getTableId(ctx)
+  const appId = ctx.appId
+
+  let deleteRequest = ctx.request.body as DeleteRows
+
+  const rowDeletes: Row[] = await processDeleteRowRequest(ctx)
+  deleteRequest.rows = rowDeletes
+
+  let { rows } = await quotas.addQuery<any>(
+    () => pickApi(tableId).bulkDestroy(ctx),
+    {
       datasourceId: tableId,
-    })
-    await quotas.removeRow()
-    response = resp.response
-    row = resp.row
+    }
+  )
+  await quotas.removeRows(rows.length)
+
+  for (let row of rows) {
     ctx.eventEmitter && ctx.eventEmitter.emitRow(`row:delete`, appId, row)
     gridSocket?.emitRowDeletion(ctx, row._id)
   }
+
+  return rows
+}
+
+async function deleteRow(ctx: UserCtx<DeleteRowRequest>) {
+  const appId = ctx.appId
+  const tableId = utils.getTableId(ctx)
+
+  let resp = await quotas.addQuery<any>(() => pickApi(tableId).destroy(ctx), {
+    datasourceId: tableId,
+  })
+  await quotas.removeRow()
+
+  ctx.eventEmitter && ctx.eventEmitter.emitRow(`row:delete`, appId, resp.row)
+  gridSocket?.emitRowDeletion(ctx, resp.row._id)
+
+  return resp
+}
+
+export async function destroy(ctx: UserCtx<DeleteRowRequest>) {
+  let response, row
   ctx.status = 200
+
+  if (isDeleteRows(ctx.request.body)) {
+    response = await deleteRows(ctx)
+  } else if (isDeleteRow(ctx.request.body)) {
+    const deleteResp = await deleteRow(ctx)
+    response = deleteResp.response
+    row = deleteResp.row
+  } else {
+    ctx.status = 400
+    response = { message: "Invalid delete rows request" }
+  }
+
   // for automations include the row that was deleted
   ctx.row = row || {}
   ctx.body = response

packages/server/src/api/routes/tests/row.spec.ts

@@ -557,6 +557,41 @@ describe("/rows", () => {
       await assertRowUsage(rowUsage - 1)
       await assertQueryUsage(queryUsage + 1)
     })
+
+    it("Should ignore malformed/invalid delete requests", async () => {
+      const rowUsage = await getRowUsage()
+      const queryUsage = await getQueryUsage()
+
+      const res = await request
+        .delete(`/api/${table._id}/rows`)
+        .send({ not: "valid" })
+        .set(config.defaultHeaders())
+        .expect("Content-Type", /json/)
+        .expect(400)
+
+      expect(res.body.message).toEqual("Invalid delete rows request")
+
+      const res2 = await request
+        .delete(`/api/${table._id}/rows`)
+        .send({ rows: 123 })
+        .set(config.defaultHeaders())
+        .expect("Content-Type", /json/)
+        .expect(400)
+
+      expect(res2.body.message).toEqual("Invalid delete rows request")
+
+      const res3 = await request
+        .delete(`/api/${table._id}/rows`)
+        .send("invalid")
+        .set(config.defaultHeaders())
+        .expect("Content-Type", /json/)
+        .expect(400)
+
+      expect(res3.body.message).toEqual("Invalid delete rows request")
+
+      await assertRowUsage(rowUsage)
+      await assertQueryUsage(queryUsage)
+    })
   })
 
   describe("fetchView", () => {