Skip to content

Commit

Permalink
Fix broken request handler (#51939)
Browse files Browse the repository at this point in the history
Hi!

In previous versions (13.4.2 and earlier) in the custom server I could
do the following:
```
const parsedUrl = parse(req.url, true);
const { pathname } = parsedUrl;

if (pathname === '/c') {
  await handle(req, res, parse('/b', true));
} else {
  await handle(req, res, parsedUrl);
}
```

Of course, you can replace `handle` with `app.render` in the attached
example, but in practice it is convenient to put the definition of
`parsedUrl` into middleware and substitute the necessary `parsedUrl` in
exceptional situations.

This was broken in #49805.

I'm not sure if this use of request handler is expected, but it has
always worked and people have used it (probably due to lack of
documentation about the difference between `app.render` and
`app.getRequestHandler()`). So the change looks breaking, and I don't
think it should appear in minor versions.

---------

Co-authored-by: Tim Neutkens <tim@timneutkens.nl>
  • Loading branch information
gwer and timneutkens authored Aug 1, 2023
1 parent ff5338c commit 629c9db
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 5 deletions.
18 changes: 13 additions & 5 deletions packages/next/src/server/next.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,23 +339,31 @@ function createServer(options: NextServerOptions): NextServer {
case 'getRequestHandler': {
return () => {
let handler: RequestHandler
return async (req: IncomingMessage, res: ServerResponse) => {
return async (
req: IncomingMessage,
res: ServerResponse,
parsedUrl?: UrlWithParsedQuery
) => {
if (shouldUseStandaloneMode) {
setupWebSocketHandler(options.httpServer, req)
const parsedUrl = url.parse(
const proxyParsedUrl = url.parse(
`http://127.0.0.1:${serverPort}${req.url}`,
true
)
if ((req?.socket as TLSSocket)?.encrypted) {
req.headers['x-forwarded-proto'] = 'https'
}
addRequestMeta(req, '__NEXT_INIT_QUERY', parsedUrl.query)
addRequestMeta(
req,
'__NEXT_INIT_QUERY',
proxyParsedUrl.query
)

await proxyRequest(req, res, parsedUrl, undefined, req)
await proxyRequest(req, res, proxyParsedUrl, undefined, req)
return
}
handler = handler || server.getRequestHandler()
return handler(req, res)
return handler(req, res, parsedUrl)
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions test/integration/custom-server/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ if (process.env.POLYFILL_FETCH) {
const { readFileSync } = require('fs')
const next = require('next')
const { join } = require('path')
const { parse } = require('url')

const dev = process.env.NODE_ENV !== 'production'
const dir = __dirname
Expand Down Expand Up @@ -68,6 +69,10 @@ app.prepare().then(() => {
}
}

if (/custom-url-with-request-handler/.test(req.url)) {
return handleNextRequests(req, res, parse('/dashboard', true))
}

handleNextRequests(req, res)
})

Expand Down
12 changes: 12 additions & 0 deletions test/integration/custom-server/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,18 @@ describe.skip.each([
expect(html).toMatch(/made it to dashboard/)
})

it('should handle custom urls with requests handler', async () => {
const html = await renderViaHTTP(
nextUrl,
'/custom-url-with-request-handler',
undefined,
{
agent,
}
)
expect(html).toMatch(/made it to dashboard/)
})

it('should contain customServer in NEXT_DATA', async () => {
const html = await renderViaHTTP(nextUrl, '/', undefined, { agent })
const $ = cheerio.load(html)
Expand Down

0 comments on commit 629c9db

Please sign in to comment.