fix: validate redirect status codes in NextResponse.redirect()#759
Conversation
NextResponse.redirect() accepted any numeric status code without validation. Passing an invalid code like 200 silently created a non-redirect response (browser ignores Location header on 200). Now throws RangeError for non-redirect status codes, matching Next.js behavior which throws on invalid codes. Valid codes: 301, 302, 303, 307, 308.
commit: |
|
/bigbonk review |
There was a problem hiding this comment.
Clean, correct fix. The implementation matches Next.js's NextResponse.redirect() validation exactly — same REDIRECTS set, same RangeError type, same error message. Tests cover the key cases.
One minor suggestion below for test completeness, but not blocking.
| expect(() => NextResponse.redirect("https://example.com", 200)).toThrow( | ||
| /Failed to execute "redirect" on "response": Invalid status code/, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Nit: the current tests only cover the init: number overload. It would be worth adding a case for the init: ResponseInit overload with an invalid status, since that's a separate code path through the ternary on line 197 of server.ts:
| }); | |
| }); | |
| it("throws RangeError when status is passed via ResponseInit object", async () => { | |
| const { NextResponse } = await import("../packages/vinext/src/shims/server.js"); | |
| expect(() => NextResponse.redirect("https://example.com", { status: 200 })).toThrow(RangeError); | |
| }); |
|
Review submitted. Here's a summary: Approved — the fix is correct and matches Next.js behavior exactly. What the PR does: Adds validation to Verification: I confirmed against the Next.js source that:
One minor suggestion: The tests only cover the |
Summary
NextResponse.redirect()accepted any status code without validationredirect(url, 200)silently created a non-redirect response — browser won't follow Location header on non-3xx statusRangeErrorfor invalid redirect status codes (anything not 301/302/303/307/308), matching Next.js behaviorTest plan
redirect(url, 200)throws RangeErrorredirect(url, 418)throws RangeErrorredirect(url, 303)succeeds (was missing coverage)redirect(url, 307)defaults correctly (existing, regression)