Skip to content

Commit

Permalink
Custom Server with bodyParser, don't parse body again in API Route (#…
Browse files Browse the repository at this point in the history
…16169)

## Why

Some users prefer to use a custom server implementation that handles body parsing. If they do this, they have no way to opt out of all body parsing in API Routes. Requests with bodies die if next's `bodyParser` is not disabled. Requests just hang forever.

Instead of adding [this config](https://nextjs.org/docs/api-routes/api-middlewares#custom-config) to every API Route, we do a simple check to avoid parsing the body twice.

Fixes #8315
Fixes #7960
  • Loading branch information
Daniel Kempner committed Dec 7, 2020
1 parent b403e9e commit 90e97b5
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 1 deletion.
2 changes: 1 addition & 1 deletion packages/next/next-server/server/api-utils.ts
Expand Up @@ -56,7 +56,7 @@ export async function apiResolver(
)

// Parsing of body
if (bodyParser) {
if (bodyParser && !apiReq.body) {
apiReq.body = await parseBody(
apiReq,
config.api && config.api.bodyParser && config.api.bodyParser.sizeLimit
Expand Down
5 changes: 5 additions & 0 deletions test/integration/api-body-parser/pages/api/index.js
@@ -0,0 +1,5 @@
export default ({ method, body }, res) => {
if (method === 'POST') {
res.status(200).json(body)
}
}
28 changes: 28 additions & 0 deletions test/integration/api-body-parser/server.js
@@ -0,0 +1,28 @@
const next = require('next')
const bodyParser = require('body-parser')
const express = require('express')

const dev = process.env.NODE_ENV !== 'production'
const dir = __dirname
const port = process.env.PORT || 3000

const app = next({ dev, dir })
const handleNextRequests = app.getRequestHandler()

app.prepare().then(() => {
const server = express()

server.use(bodyParser.json({ limit: '5mb' }))

server.all('*', (req, res) => {
handleNextRequests(req, res)
})

server.listen(port, (err) => {
if (err) {
throw err
}

console.log(`> Ready on http://localhost:${port}`)
})
})
72 changes: 72 additions & 0 deletions test/integration/api-body-parser/test/index.test.js
@@ -0,0 +1,72 @@
/* eslint-env jest */

import { join } from 'path'
import {
killApp,
findPort,
launchApp,
fetchViaHTTP,
initNextServerScript,
} from 'next-test-utils'
import clone from 'clone'
import getPort from 'get-port'

jest.setTimeout(1000 * 60 * 2)
const appDir = join(__dirname, '../')
let appPort

let app
let server
jest.setTimeout(1000 * 60 * 2)

const context = {}

function runTests() {
it('should parse JSON body', async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort, {})
const data = await makeRequest()
expect(data).toEqual([{ title: 'Nextjs' }])
killApp(app)
})

it('should not throw if request body is already parsed in custom middleware', async () => {
await startServer()
const data = await makeRequest()
expect(data).toEqual([{ title: 'Nextjs' }])
killApp(server)
})
}

async function makeRequest() {
const data = await fetchViaHTTP(appPort, '/api', null, {
method: 'POST',
headers: {
'Content-Type': 'application/json; charset=utf-8',
},
body: JSON.stringify([{ title: 'Nextjs' }]),
}).then((res) => res.ok && res.json())

return data
}

const startServer = async (optEnv = {}, opts) => {
const scriptPath = join(appDir, 'server.js')
context.appPort = appPort = await getPort()
const env = Object.assign(
{},
clone(process.env),
{ PORT: `${appPort}` },
optEnv
)

server = await initNextServerScript(
scriptPath,
/ready on/i,
env,
/ReferenceError: options is not defined/,
opts
)
}

runTests()

0 comments on commit 90e97b5

Please sign in to comment.