vercel / next.js

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

NextCustomServer overwrites req.url with internal NextJS path #55341

Open tills13 opened 1 year ago

tills13 commented 1 year ago

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

https://github.com/tills13/nextjs-req-url-reproduction

To Reproduce

  1. install deps (npm ci)
  2. build next (npx next build)
  3. run server (node server.js)
  4. navigate to http://localhost:3000 (any path)
  5. note that the URL is the internal NextJS URL instead of the expected user-facing URL

Current vs. Expected behavior

The expected behaviour is that, for Custom Servers, at least, the request URL is untouched as you might want to render a page at a different path than what the URL path is e.g. /blog/some-slug/ -> /pages/blog_post.js

Our sites do not have uniform URLs (both /blog/some-slug/ and /some-slug/ might refer to a blog post or a static page and we don't know until we resolve the object from our CMS) so we cannot use file system routing.

Verify canary release

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #1 SMP Fri Jan 27 02:56:13 UTC 2023
    Binaries:
      Node: 16.20.0
      npm: 8.19.4
      Yarn: N/A
      pnpm: N/A
    Relevant Packages:
      next: 13.4.20-canary.27
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0
      typescript: N/A
    Next.js Config:
      output: N/A

Which area(s) are affected? (Select all that apply)

Routing (next/router, next/navigation, next/link)

Additional context

Seems like the issue was introduce in https://github.com/vercel/next.js/pull/49805 or https://github.com/vercel/next.js/pull/53523

tills13 commented 1 year ago

Gating https://github.com/vercel/next.js/blob/canary/packages/next/src/server/next.ts#L348-L352 on useFileSystemRouting might be acceptable, but really imo there's no reason why req.url should ever be touched.

LMK which path you think is best and I can throw up a PR.

tills13 commented 1 year ago

Playing around with my example, actually, it seems like I'm either missing something or misunderstanding something. If I do set useFileSystemPublicRoutes to false, nextjs cannot find the page in pages. This seems contrary to the Custom Server examples.

Captured in https://github.com/vercel/next.js/issues/55344

tills13 commented 1 year ago

Theoretically, this messes with the trailingSlash config option, as well.

app.render(req, res, '/internal_page')

When I'm using a custom server and custom routes, the page here doesn't need the trailing slash. What I'm seeing in our production app, and trying to build a reproduction case for, is that the redirect generated is for the internal URL not the external URL (e.g. it would be 308 /internal_page/ for the above instead of for the external-facing URL).

bobaaaaa commented 11 months ago

We run into the same issue. Can someone from the nextjs team please say if this is intended or a bug? @shuding @timneutkens @leerob

philipheinser commented 11 months ago

Reported this before caused a lot of problems with production deploys. https://github.com/vercel/next.js/issues/54495