vercel / next.js

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

`DecodeError` from invalid URI causes 500 with middleware (vs 400 without) #36964

Closed merrywhether closed 2 years ago

merrywhether commented 2 years ago

Verify canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 21.4.0: Fri Mar 18 00:46:32 PDT 2022; root:xnu-8020.101.4~15/RELEASE_ARM64_T6000
    Binaries:
      Node: 16.14.2
      npm: 8.5.0
      Yarn: 1.22.18
      pnpm: N/A
    Relevant packages:
      next: 12.1.7-canary.6
      react: 18.1.0
      react-dom: 18.1.0

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

When an invalid URI is requested against a dynamic route, Next will hit a DecodeError and ultimately return a 400. If a custom _error page is provided, the res object provided to _error.getInitialProps will also have statusCode: 400.

Changing nothing else, if middleware is added the response is now a 500, yielding this stack.

DecodeError: failed to decode param
      at decode (.../test/node_modules/next/dist/shared/lib/router/utils/route-matcher.js:18:23)
      at .../test/node_modules/next/dist/shared/lib/router/utils/route-matcher.js:29:21
      at Array.forEach (<anonymous>)
      at Object.match (.../test/node_modules/next/dist/shared/lib/router/utils/route-matcher.js:22:29)
      at DevServer.runMiddleware (.../test/node_modules/next/dist/server/next-server.js:837:50)
      at async DevServer.runMiddleware (.../test/node_modules/next/dist/server/dev/next-dev-server.js:435:28)
      at async Object.fn (.../test/node_modules/next/dist/server/next-server.js:710:30)
      at async Router.execute (.../test/node_modules/next/dist/server/router.js:226:36)
      at async DevServer.run (.../test/node_modules/next/dist/server/base-server.js:636:29)
      at async DevServer.run (.../test/node_modules/next/dist/server/dev/next-dev-server.js:483:20)

(Originates from here, and middleware: true is also attached to this error object.)

This 500 is both the status code for the default UI, and if a custom _error page is provided, the res object provided to _error.getInitialProps will now have statusCode: 500.

Expected Behavior

The simple presence of middleware should not effect the outcome of this request, and both situations should yield 400 status codes to either default UI or custom application handling.

Without custom error pages, Next should return a 400 status code with the "400 Bad Request" default UI.

With custom error pages, Next should yield a res object to _error.getInitialProps with statusCode: 400.

I'm guessing that the middleware logic needs the same special casing of DecodeError as found elsewhere: https://github.com/vercel/next.js/blob/canary/packages/next/server/base-server.ts#L579

To Reproduce

Minimal repro here: https://github.com/merrywhether/next-uri-repro

Steps:

  1. Start a new Next project and start app (build && start is necessary to see UI outcomes of errors locally, but dev also works)

  2. Add a dynamic route eg pages/[test].js:

    
    import { useRouter } from 'next/router';

const Post = () => { const { query: { test }, } = useRouter();

return

Test: {test}

; };

export default Post;


3. Make a good request (eg `/hello`) and see "Test: hello" with 200 status

4. Make a bad request (eg `/%c0`) and see "400 | Bad Request" default UI with 400 status

5. Add `pages/_error.js`:

export const ErrorPage = ({ statusCode }) => { return

Error {statusCode}
; };

ErrorPage.getInitialProps = (ctx) => { return { statusCode: ctx.res?.statusCode }; };

export default ErrorPage;


6. Make same bad request and see "Error 400" custom UI with 400 status

7. Add `pages/_middleware.js`:

import { NextResponse } from 'next/server';

export const middleware = () => { return NextResponse.next(); };



8. Make same bad request and see "Error 500" with 500 status

9. Move/remove `_error.js`, make same bad request, and see "500 | Internal Server Error" default UI with 500 status
github-actions[bot] commented 2 years ago

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.