vercel / next.js

The React Framework
https://nextjs.org
MIT License
124.94k stars 26.68k forks source link

Incorrect response status code when using NextResponse.rewrite(url, { status }) in middleware #50155

Open stefee opened 1 year ago

stefee commented 1 year ago

Verify canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.5.0: Mon Apr 24 20:52:24 PDT 2023; root:xnu-8796.121.2~5/RELEASE_ARM64_T6000
    Binaries:
      Node: 18.12.1
      npm: 8.19.2
      Yarn: N/A
      pnpm: N/A
    Relevant packages:
      next: 13.4.4-canary.0
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 4.9.5

Which area(s) of Next.js are affected? (leave empty if unsure)

Middleware / Edge (API routes, runtime)

Link to the code that reproduces this issue or a replay of the bug

https://github.com/stefee/next-res-status-repro/commit/48b14d833ee8bc546a427b5d5fffd6bb9efbced9

To Reproduce

Run

npm run dev

Open the application

Describe the Bug

Response status code is 200

Expected Behavior

Response status code should be 403

Which browser are you using? (if relevant)

N/A

How are you deploying your application? (if relevant)

next dev, standalone (Docker)

kijikunnn commented 1 year ago

I apologize if my understanding is incorrect.

The following code is part of NextResponse. From what I see in the code, rewrite does not seem to return a status. It appears that if a page exists, it would return a 200 status, and if it does not, it would return a 404 status. If a response with a specific status code is necessary, shouldn't we be using redirect instead?

https://github.com/vercel/next.js/blob/62af2007ce78fdbff33013a8145efbcacbf6b8e2/packages/next/src/server/web/spec-extension/response.ts#L79-L106

stefee commented 1 year ago

@kijikunnn Thanks for your response 🙏🏻 If a user is not authorized to access a page but we do not want to redirect to a different route, how do you suggest we achieve that?

The advantage of rewrite instead of redirect is that we can display the "Not Authorized" page and respond with a 403 status whilst maintaining the same URL. This way the user can hit "Refresh" in the browser once they have been granted access. A common example of this is Google Docs if you have not been granted permissions to view a document yet, but once you have been granted then you can hit refresh to view the document.

stefee commented 1 year ago

P.S. the main advantage of using the 403 status code instead of 200 is that we can monitor the number of 4xx status code responses in our CDN to detect suspicious activity. The CDN can also handle caching of the page differently for example.

jknight12882 commented 1 year ago

Agree with this. Showing different content while retaining the url is fairly common. Taking the site down for maintenance for example. You don't want to navigate users (or more importantly, crawlers) away from the url. You want to give them a 503 and a graceful landing page

aslakhellesoy commented 1 year ago

I agree this should be supported. NextResponse.rewrite currently allows a 2nd argument of type MiddlewareResponseInit, which has a status property. Why does it even allow this 2nd argument if anything you put in it is ignored?

astateful commented 1 year ago

This should definitely be supported. I tried it myself just now and can confirm it is not working. We are SEO driven company and therefore need to, among other things, return 410 code on some pages to provide bots with a more graceful error message, as well as 451 code (unavailable for legal reasons). So being able to change status code is also critical in a business sense.

isaac-martin commented 1 year ago

yes really needing to pass a 451 to a rewrite or redirect here for unavailalbe regions. Even the redirect won't accept a 451 status code and fails with invalid code message

webgears-cd commented 1 year ago

@isaac-martin What I actually realised is that it is still possible to return custom error codes, however it is not possible to display a NextJS rendered page in this case, only static content. For example:


if (some_condition) {
  return new NextResponse(
    JSON.stringify({ message: 'service not available' }),
    {
      status: 451,
      headers: { 'content-type': 'application/json' }
    }
  );
}```

But for business reasons we more than likely do not want to return such a page to a client. So basically, to clarify matters, we need to be able to redirect or rewrite with a custom error code to a NextJS rendered page.
gerkim62 commented 1 month ago

did this get fixed yet?