Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[actions] Enforce body limit using Transform stream #64694

Merged
merged 2 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We didn't have the types here before, this isn't bundled, but enhances the code that uses busboy with some types.

"@types/cheerio": "0.22.16",
"@types/cookie": "0.3.3",
"@types/cross-spawn": "6.0.0",
Expand Down
92 changes: 63 additions & 29 deletions packages/next/src/server/app-render/action-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,11 @@ type ServerModuleMap = Record<
| undefined
>

type ServerActionsConfig = {
bodySizeLimit?: SizeLimit
allowedOrigins?: string[]
}

export async function handleAction({
req,
res,
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions test/e2e/app-dir/actions/account-for-overhead.js
Original file line number Diff line number Diff line change
@@ -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)
}
84 changes: 42 additions & 42 deletions test/e2e/app-dir/actions/app-action-size-limit-invalid.test.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -9,6 +10,7 @@ createNextDescribe(
{
files: __dirname,
skipDeployment: true,
skipStart: true,
dependencies: {
react: 'latest',
'react-dom': 'latest',
Expand All @@ -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',
Expand All @@ -32,9 +51,8 @@ createNextDescribe(
}
`
)
await next.stop()
try {
await next.build()
await next.start()
} catch {}
expect(next.cliOutput).toContain(CONFIG_ERROR)
})
Expand All @@ -50,9 +68,8 @@ createNextDescribe(
}
`
)
await next.stop()
try {
await next.build()
await next.start()
} catch {}
expect(next.cliOutput).toContain(CONFIG_ERROR)
})
Expand All @@ -68,9 +85,8 @@ createNextDescribe(
}
`
)
await next.stop()
try {
await next.build()
await next.start()
} catch {}
expect(next.cliOutput).toContain(CONFIG_ERROR)
})
Expand All @@ -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 () {
Expand All @@ -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)}`)
})
})
}
}
Expand Down
5 changes: 4 additions & 1 deletion test/e2e/app-dir/actions/app/file/form.js
Original file line number Diff line number Diff line change
@@ -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)}>
Expand Down
8 changes: 5 additions & 3 deletions test/e2e/app-dir/actions/app/form/page.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { accountForOverhead } from '../../account-for-overhead'

async function action(formData) {
'use server'
const payload = formData.get('payload').toString()
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Loading