[actions] Enforce body limit using Transform stream (#64694)

### What? This ensures that the body limit option is enforced on all request bodies sent to the Node.js runtime, not just the multipart field size limits. ### Why? The documentation states that this should limit the body size, previously it only limited the field size. ### How? This uses a `Transform` stream from Node.js. [Based on my benchmarks](https://gist.github.com/wyattjoh/c470d98095da2f95f5920396aba2a206) using the transform stream added next to no overhead, yet it did simplify the implementation quite a bit. Assuming this is due to the already performant stream support within Node.js. Closes NEXT-3151
vercel · Apr 18, 2024 · 64e2ffc · 64e2ffc
1 parent 00d1189
commit 64e2ffc
Show file tree

Hide file tree

Showing 7 changed files with 135 additions and 75 deletions.
diff --git a/package.json b/package.json
@@ -95,6 +95,7 @@
     "@swc/helpers": "0.5.5",
     "@testing-library/jest-dom": "6.1.2",
     "@testing-library/react": "13.0.0",
+    "@types/busboy": "1.5.3",
     "@types/cheerio": "0.22.16",
     "@types/cookie": "0.3.3",
     "@types/cross-spawn": "6.0.0",

diff --git a/packages/next/src/server/app-render/action-handler.ts b/packages/next/src/server/app-render/action-handler.ts
@@ -353,6 +353,11 @@ type ServerModuleMap = Record<
   | undefined
 >
 
+type ServerActionsConfig = {
+  bodySizeLimit?: SizeLimit
+  allowedOrigins?: string[]
+}
+
 export async function handleAction({
   req,
   res,
@@ -371,10 +376,7 @@ export async function handleAction({
   generateFlight: GenerateFlight
   staticGenerationStore: StaticGenerationStore
   requestStore: RequestStore
-  serverActions?: {
-    bodySizeLimit?: SizeLimit
-    allowedOrigins?: string[]
-  }
+  serverActions?: ServerActionsConfig
   ctx: AppRenderContext
 }): Promise<
   | undefined
@@ -552,11 +554,12 @@ export async function handleAction({
       ) {
         // Use react-server-dom-webpack/server.edge
         const { decodeReply, decodeAction, decodeFormState } = ComponentMod
-
         if (!req.body) {
           throw new Error('invariant: Missing request body.')
         }
 
+        // TODO: add body limit
+
         if (isMultipartAction) {
           // TODO-APP: Add streaming support
           const formData = await req.request.formData()
@@ -619,28 +622,72 @@ export async function handleAction({
           decodeFormState,
         } = require(`./react-server.node`)
 
+        const { Transform } =
+          require('node:stream') as typeof import('node:stream')
+
+        const defaultBodySizeLimit = '1 MB'
+        const bodySizeLimit =
+          serverActions?.bodySizeLimit ?? defaultBodySizeLimit
+        const bodySizeLimitBytes =
+          bodySizeLimit !== defaultBodySizeLimit
+            ? (
+                require('next/dist/compiled/bytes') as typeof import('bytes')
+              ).parse(bodySizeLimit)
+            : 1024 * 1024 // 1 MB
+
+        let size = 0
+        const body = req.body.pipe(
+          new Transform({
+            transform(chunk, encoding, callback) {
+              size += Buffer.byteLength(chunk, encoding)
+              if (size > bodySizeLimitBytes) {
+                const { ApiError } = require('../api-utils')
+
+                callback(
+                  new ApiError(
+                    413,
+                    `Body exceeded ${bodySizeLimit} limit.
+                To configure the body size limit for Server Actions, see: https://nextjs.org/docs/app/api-reference/next-config-js/serverActions#bodysizelimit`
+                  )
+                )
+                return
+              }
+
+              callback(null, chunk)
+            },
+          })
+        )
+
         if (isMultipartAction) {
           if (isFetchAction) {
-            const readableLimit = serverActions?.bodySizeLimit ?? '1 MB'
-            const limit = require('next/dist/compiled/bytes').parse(
-              readableLimit
-            )
-            const busboy = require('busboy')
-            const bb = busboy({
+            const busboy = (require('busboy') as typeof import('busboy'))({
               headers: req.headers,
-              limits: { fieldSize: limit },
+              limits: { fieldSize: bodySizeLimitBytes },
             })
-            req.body.pipe(bb)
 
-            bound = await decodeReplyFromBusboy(bb, serverModuleMap)
+            body.pipe(busboy)
+
+            bound = await decodeReplyFromBusboy(busboy, serverModuleMap)
           } else {
             // React doesn't yet publish a busboy version of decodeAction
             // so we polyfill the parsing of FormData.
             const fakeRequest = new Request('http://localhost', {
               method: 'POST',
               // @ts-expect-error
               headers: { 'Content-Type': contentType },
-              body: req.stream(),
+              body: new ReadableStream({
+                start: (controller) => {
+                  body.on('data', (chunk) => {
+                    controller.enqueue(new Uint8Array(chunk))
+                  })
+                  body.on('end', () => {
+                    controller.close()
+                  })
+                  body.on('error', (err) => {
+                    controller.error(err)
+                  })
+                },
+              }),
               duplex: 'half',
             })
             const formData = await fakeRequest.formData()
@@ -667,26 +714,13 @@ export async function handleAction({
             }
           }
 
-          const chunks = []
-
+          const chunks: Buffer[] = []
           for await (const chunk of req.body) {
             chunks.push(Buffer.from(chunk))
           }
 
           const actionData = Buffer.concat(chunks).toString('utf-8')
 
-          const readableLimit = serverActions?.bodySizeLimit ?? '1 MB'
-          const limit = require('next/dist/compiled/bytes').parse(readableLimit)
-
-          if (actionData.length > limit) {
-            const { ApiError } = require('../api-utils')
-            throw new ApiError(
-              413,
-              `Body exceeded ${readableLimit} limit.
-To configure the body size limit for Server Actions, see: https://nextjs.org/docs/app/api-reference/next-config-js/serverActions#bodysizelimit`
-            )
-          }
-
           if (isURLEncodedAction) {
             const formData = formDataFromSearchQueryString(actionData)
             bound = await decodeReply(formData, serverModuleMap)

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/test/e2e/app-dir/actions/account-for-overhead.js b/test/e2e/app-dir/actions/account-for-overhead.js
@@ -0,0 +1,11 @@
+/**
+ * This function accounts for the overhead of encoding the data to be sent
+ * over the network via a multipart request.
+ *
+ * @param {number} megaBytes
+ * @returns {number}
+ */
+export function accountForOverhead(megaBytes) {
+  // We are sending {megaBytes} - 5% to account for encoding overhead
+  return Math.floor(1024 * 1024 * megaBytes * 0.95)
+}
diff --git a/test/e2e/app-dir/actions/app-action-size-limit-invalid.test.ts b/test/e2e/app-dir/actions/app-action-size-limit-invalid.test.ts
@@ -1,5 +1,6 @@
 import { createNextDescribe } from 'e2e-utils'
-import { check } from 'next-test-utils'
+import { retry } from 'next-test-utils'
+import { accountForOverhead } from './account-for-overhead'
 
 const CONFIG_ERROR =
   'Server Actions Size Limit must be a valid number or filesize format larger than 1MB'
@@ -9,6 +10,7 @@ createNextDescribe(
   {
     files: __dirname,
     skipDeployment: true,
+    skipStart: true,
     dependencies: {
       react: 'latest',
       'react-dom': 'latest',
@@ -21,6 +23,23 @@ createNextDescribe(
       return
     }
 
+    const logs: string[] = []
+
+    beforeAll(() => {
+      const onLog = (log: string) => {
+        logs.push(log.trim())
+      }
+
+      next.on('stdout', onLog)
+      next.on('stderr', onLog)
+    })
+
+    afterEach(async () => {
+      logs.length = 0
+
+      await next.stop()
+    })
+
     it('should error if serverActions.bodySizeLimit config is a negative number', async function () {
       await next.patchFile(
         'next.config.js',
@@ -32,9 +51,8 @@ createNextDescribe(
       }
       `
       )
-      await next.stop()
       try {
-        await next.build()
+        await next.start()
       } catch {}
       expect(next.cliOutput).toContain(CONFIG_ERROR)
     })
@@ -50,9 +68,8 @@ createNextDescribe(
       }
       `
       )
-      await next.stop()
       try {
-        await next.build()
+        await next.start()
       } catch {}
       expect(next.cliOutput).toContain(CONFIG_ERROR)
     })
@@ -68,9 +85,8 @@ createNextDescribe(
       }
       `
       )
-      await next.stop()
       try {
-        await next.build()
+        await next.start()
       } catch {}
       expect(next.cliOutput).toContain(CONFIG_ERROR)
     })
@@ -87,35 +103,27 @@ createNextDescribe(
       }
       `
         )
-        await next.build()
         await next.start()
 
-        const logs: string[] = []
-        next.on('stdout', (log) => {
-          logs.push(log)
-        })
-        next.on('stderr', (log) => {
-          logs.push(log)
-        })
-
         const browser = await next.browser('/file')
         await browser.elementByCss('#size-1mb').click()
 
-        await check(() => {
-          return logs.some((log) => log.includes('size = 1048576')) ? 'yes' : ''
-        }, 'yes')
+        await retry(() => {
+          expect(logs).toContainEqual(`size = ${accountForOverhead(1)}`)
+        })
 
         await browser.elementByCss('#size-2mb').click()
 
-        await check(() => {
-          const fullLog = logs.join('')
-          return fullLog.includes('[Error]: Body exceeded 1.5mb limit') &&
-            fullLog.includes(
+        await retry(() => {
+          expect(logs).toContainEqual(
+            expect.stringContaining('[Error]: Body exceeded 1.5mb limit')
+          )
+          expect(logs).toContainEqual(
+            expect.stringContaining(
               'To configure the body size limit for Server Actions, see'
             )
-            ? 'yes'
-            : ''
-        }, 'yes')
+          )
+        })
       })
 
       it('should respect the size set in serverActions.bodySizeLimit when submitting form', async function () {
@@ -129,29 +137,21 @@ createNextDescribe(
       }
       `
         )
-        await next.stop()
-        await next.build()
-        await next.start()
 
-        const logs: string[] = []
-        next.on('stdout', (log) => {
-          logs.push(log)
-        })
-        next.on('stderr', (log) => {
-          logs.push(log)
-        })
+        await next.start()
 
         const browser = await next.browser('/form')
         await browser.elementByCss('#size-1mb').click()
 
-        await check(() => {
-          return logs.some((log) => log.includes('size = 1048576')) ? 'yes' : ''
-        }, 'yes')
+        await retry(() => {
+          expect(logs).toContainEqual(`size = ${accountForOverhead(1)}`)
+        })
 
         await browser.elementByCss('#size-2mb').click()
-        await check(() => {
-          return logs.some((log) => log.includes('size = 2097152')) ? 'yes' : ''
-        }, 'yes')
+
+        await retry(() => {
+          expect(logs).toContainEqual(`size = ${accountForOverhead(2)}`)
+        })
       })
     }
   }

diff --git a/test/e2e/app-dir/actions/app/file/form.js b/test/e2e/app-dir/actions/app/file/form.js
@@ -1,7 +1,10 @@
 'use client'
+import { accountForOverhead } from '../../account-for-overhead'
 
 export default function Form({ action }) {
-  const submit = (n) => action('a'.repeat(1024 * 1024 * n))
+  const submit = (megaBytes) =>
+    action('a'.repeat(accountForOverhead(megaBytes)))
+
   return (
     <>
       <button id="size-1mb" onClick={() => submit(1)}>

diff --git a/test/e2e/app-dir/actions/app/form/page.js b/test/e2e/app-dir/actions/app/form/page.js
@@ -1,3 +1,5 @@
+import { accountForOverhead } from '../../account-for-overhead'
+
 async function action(formData) {
   'use server'
   const payload = formData.get('payload').toString()
@@ -11,7 +13,7 @@ export default function Page() {
         <input
           type="hidden"
           name="payload"
-          value={'a'.repeat(1024 * 1024 * 1)}
+          value={'a'.repeat(accountForOverhead(1))}
         />
         <button type="submit" id="size-1mb">
           SUBMIT 1mb
@@ -21,7 +23,7 @@ export default function Page() {
         <input
           type="hidden"
           name="payload"
-          value={'a'.repeat(1024 * 1024 * 2)}
+          value={'a'.repeat(accountForOverhead(2))}
         />
         <button type="submit" id="size-2mb">
           SUBMIT 2mb
@@ -31,7 +33,7 @@ export default function Page() {
         <input
           type="hidden"
           name="payload"
-          value={'a'.repeat(1024 * 1024 * 3)}
+          value={'a'.repeat(accountForOverhead(3))}
         />
         <button type="submit" id="size-3mb">
           SUBMIT 3mb