Skip to content

Commit

Permalink
Call res.write('') inside apiRes.redirect() helper to prevent an edge…
Browse files Browse the repository at this point in the history
… case (#20461)

This PR is a small follow-up to #14705. It saves Next.js users from falling into a [pretty nasty trap](nodejs/node#36620) in which I ended up last Friday. It took more than two days to investigate what was going on, so I hope I'm the last person who’s doing it 😅

Next.js-specific MWE: https://github.com/kachkaev/hanging-response-in-next-via-redirect-plus-compression (needs to be ran locally using Node 14.0.0+).

> <img width="521" alt="Screenshot 2020-12-24 at 20 50 00" src="https://user-images.githubusercontent.com/608862/103105989-a9b8dc00-4629-11eb-9be3-5108755604bf.png">

To reproduce the bug I’m fixing:

1. Pick a large http body size (64 or 128 KB)
1. Check _Call res.end() after res.redirect() in /api/redirect_
1. Navigate to a heavy page or an api handler via redirect
1. Observe that the http response is never finished.

If you set `compress` to `false` in `next.config.js` or pick a small payload size (< `zlib.Z_DEFAULT_CHUNK` after compression), the bug will not be observed. This is explained by the use of `res.on("drain", ...)` [by the `compression` package](https://github.com/expressjs/compression/blob/3fea81d0eaed1eb872bf3e0405f20d9e175ab2cf/index.js#L193-L218). The package itself is not the reason for an issue though, it seems to be in the Node’s built-in `http` package.

I’m happy to provide more info or GitHub CI to the MWE if needed. I was also thinking of adding some Next.js-specific testing, but could not come up with a compact and clear test plan. Happy to do this if there are any ideas.

cc @botv (author of #14705)
  • Loading branch information
kachkaev authored Jan 4, 2021
1 parent 74166ea commit 7076758
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion packages/next/next-server/server/api-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,9 @@ export function redirect(
`Invalid redirect arguments. Please use a single argument URL, e.g. res.redirect('/destination') or use a status code and URL, e.g. res.redirect(307, '/destination').`
)
}
res.writeHead(statusOrUrl, { Location: url }).end()
res.writeHead(statusOrUrl, { Location: url })
res.write('')
res.end()
return res
}

Expand Down

0 comments on commit 7076758

Please sign in to comment.