Skip to content

Commit 35d6fdf

Browse files
lucasadrianofijjk
andauthored
fix(nodejs-middleware): await for body cloning to be properly finalized (#85418)
Fixes #85416. This is a follow up to #77662, where the Node.js middleware was fixed to also support reading (and duplicating) the body's request before being passed to the server action. However, in that PR, an `await` was missed while calling the [.finalize()](https://github.com/vercel/next.js/blob/1b45163bce3dab30d7a79dff95238b84016b54c2/packages/next/src/server/body-streams.ts#L39) method, which returns a Promise. That missing `await` would sometimes cause a bug, where the server action would be invoked before the body was completely cloned, causing the bug mentioned in the issue above. --------- Co-authored-by: JJ Kasper <jj@jjsweb.site>
1 parent 4e5915d commit 35d6fdf

File tree

12 files changed

+237
-3
lines changed

12 files changed

+237
-3
lines changed

packages/next/src/server/next-server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1750,7 +1750,7 @@ export default class NextNodeServer extends BaseServer<
17501750
})
17511751
} finally {
17521752
if (hasRequestBody) {
1753-
requestData.body.finalize()
1753+
await requestData.body.finalize()
17541754
}
17551755
}
17561756
} else {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use server'
2+
3+
/**
4+
* @param {number[]} largeJson
5+
*/
6+
export async function submitLargePayload(largeJson) {
7+
return {
8+
success: true,
9+
count: largeJson.length,
10+
firstId: largeJson[0],
11+
lastId: largeJson[largeJson.length - 1],
12+
}
13+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use client'
2+
3+
import { useMemo, useState } from 'react'
4+
import { submitLargePayload } from './actions'
5+
6+
export default function Page() {
7+
const [result, setResult] = useState(null)
8+
9+
const largePayload = useMemo(
10+
() => new Array(10 * 1024).fill(null).map((_, idx) => idx),
11+
[]
12+
)
13+
14+
const handleSubmit = async () => {
15+
const res = await submitLargePayload(largePayload)
16+
setResult(res)
17+
}
18+
19+
return (
20+
<div>
21+
<button onClick={handleSubmit} id="submit-large">
22+
Submit Large Payload
23+
</button>
24+
{result && <div id="result">{JSON.stringify(result)}</div>}
25+
</div>
26+
)
27+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import React from 'react'
2+
3+
export default function DefaultLayout({
4+
children,
5+
}: {
6+
children: React.ReactNode
7+
}) {
8+
return (
9+
<html>
10+
<head />
11+
<body>{children}</body>
12+
</html>
13+
)
14+
}

test/e2e/app-dir/actions/middleware-node.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ export async function middleware(req) {
77
return NextResponse.rewrite(req.nextUrl)
88
}
99

10+
if (req.method === 'POST' && req.nextUrl.pathname.includes('body-finalize')) {
11+
const body = await req.json()
12+
13+
console.log(
14+
'Middleware - Body length: %d bytes',
15+
new TextEncoder().encode(JSON.stringify(body)).length
16+
)
17+
}
18+
1019
return NextResponse.next()
1120
}
1221

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import { nextTestSetup } from 'e2e-utils'
2+
import {
3+
findPort,
4+
getFullUrl,
5+
initNextServerScript,
6+
killApp,
7+
retry,
8+
} from 'next-test-utils'
9+
import webdriver from 'next-webdriver'
10+
import path from 'node:path'
11+
import fs from 'fs-extra'
12+
import os from 'os'
13+
14+
describe('app-dir action body finalize with nodejs middleware and output-standalone', () => {
15+
const { next } = nextTestSetup({
16+
files: __dirname,
17+
skipStart: true,
18+
})
19+
20+
let server: any
21+
let appPort: number
22+
let tmpFolder: string
23+
24+
beforeAll(async () => {
25+
tmpFolder = path.join(os.tmpdir(), 'next-standalone-' + Date.now())
26+
await fs.mkdirp(tmpFolder)
27+
28+
await next.build()
29+
await next.patchFile(
30+
'.next/standalone/node_modules/next/dist/server/body-streams.js',
31+
(content) => {
32+
return content.replace(
33+
'async finalize () {',
34+
'async finalize () { \nawait new Promise((resolve) => setTimeout(resolve, (Math.random() * 1000) + 1000));\n'
35+
)
36+
}
37+
)
38+
39+
const distFolder = path.join(tmpFolder, 'test')
40+
await fs.move(path.join(next.testDir, '.next/standalone'), distFolder)
41+
await fs.move(
42+
path.join(next.testDir, '.next/static'),
43+
path.join(distFolder, '.next/static')
44+
)
45+
46+
const testServer = path.join(distFolder, 'server.js')
47+
appPort = await findPort()
48+
server = await initNextServerScript(
49+
testServer,
50+
/- Local:/,
51+
{
52+
...process.env,
53+
PORT: appPort.toString(),
54+
},
55+
undefined,
56+
{
57+
cwd: distFolder,
58+
}
59+
)
60+
})
61+
62+
afterAll(async () => {
63+
if (server) await killApp(server)
64+
if (!process.env.NEXT_TEST_SKIP_CLEANUP) {
65+
await fs.remove(tmpFolder).catch(console.error)
66+
}
67+
})
68+
69+
it('should handle large payload through server action after nodejs middleware with delayed body finalize', async () => {
70+
const browser = await webdriver(getFullUrl(appPort, '/body-finalize'), '')
71+
72+
try {
73+
await browser.elementById('submit-large').click()
74+
await retry(async () => {
75+
const resultText = await browser.elementById('result').text()
76+
const result = JSON.parse(resultText)
77+
78+
expect(result.success).toBe(true)
79+
expect(result.count).toBe(10 * 1024)
80+
expect(result.firstId).toBe(0)
81+
expect(result.lastId).toBe(10 * 1024 - 1)
82+
})
83+
} finally {
84+
await browser.close()
85+
}
86+
})
87+
})
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use server'
2+
3+
/**
4+
* @param {number[]} largeJson
5+
*/
6+
export async function submitLargePayload(largeJson) {
7+
return {
8+
success: true,
9+
count: largeJson.length,
10+
firstId: largeJson[0],
11+
lastId: largeJson[largeJson.length - 1],
12+
}
13+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use client'
2+
3+
import { useMemo, useState } from 'react'
4+
import { submitLargePayload } from './actions'
5+
6+
export default function Page() {
7+
const [result, setResult] = useState(null)
8+
9+
const largePayload = useMemo(
10+
() => new Array(10 * 1024).fill(null).map((_, idx) => idx),
11+
[]
12+
)
13+
14+
const handleSubmit = async () => {
15+
const res = await submitLargePayload(largePayload)
16+
setResult(res)
17+
}
18+
19+
return (
20+
<div>
21+
<button onClick={handleSubmit} id="submit-large">
22+
Submit Large Payload
23+
</button>
24+
{result && <div id="result">{JSON.stringify(result)}</div>}
25+
</div>
26+
)
27+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
export default function RootLayout({ children }) {
2+
return (
3+
<html>
4+
<head />
5+
<body>{children}</body>
6+
</html>
7+
)
8+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Ensure that https://github.com/vercel/next.js/issues/56286 is fixed.
2+
import { NextResponse } from 'next/server'
3+
4+
export async function middleware(req) {
5+
if (req.nextUrl.pathname.includes('rewrite-to-static-first')) {
6+
req.nextUrl.pathname = '/static/first'
7+
return NextResponse.rewrite(req.nextUrl)
8+
}
9+
10+
if (req.method === 'POST' && req.nextUrl.pathname.includes('body-finalize')) {
11+
const body = await req.json()
12+
13+
console.log(
14+
'Middleware - Body length: %d bytes',
15+
new TextEncoder().encode(JSON.stringify(body)).length
16+
)
17+
}
18+
19+
return NextResponse.next()
20+
}
21+
22+
/**
23+
* @type {import('next/server').ProxyConfig}
24+
*/
25+
export const config = {
26+
runtime: 'nodejs',
27+
}

0 commit comments

Comments
 (0)