next.js icon indicating copy to clipboard operation
next.js copied to clipboard

TypeError: Invalid URL when request: NextRequest is called in app router api route in production

Open Lodimup opened this issue 2 years ago • 11 comments

Link to the code that reproduces this issue

https://codesandbox.io/p/sandbox/peaceful-smoke-yw6tz9?file=%2Fapp%2F(sitemap)%2Fsitemap%2F%5Bslug%5D%2Froute.ts%3A6%2C1

To Reproduce

// app/(sitemap)/sitemap/[slug]/route.ts
import { type NextRequest } from 'next/server';

export function GET(
	request: NextRequest,
	{ params }: { params: { slug: string } }
) {
	console.log('fn body called')
...
}

Current vs. Expected behavior

Actual result

code should reach console.log('fn body called') but instead we get

docker compose logs -f frontend
kruaco20-frontend-3  | 
kruaco20-frontend-3  | > [email protected] start
kruaco20-frontend-3  | > next start
kruaco20-frontend-3  | 
kruaco20-frontend-3  |    ▲ Next.js 14.0.3
kruaco20-frontend-3  |    - Local:        http://localhost:3000
kruaco20-frontend-3  | 
kruaco20-frontend-3  |  ✓ Ready in 1110ms
kruaco20-frontend-3  |  ⨯ TypeError: Invalid URL
kruaco20-frontend-3  |     at new URL (node:internal/url:775:36)
kruaco20-frontend-3  |     at NextRequestAdapter.fromNodeNextRequest (/app/node_modules/next/dist/server/web/spec-extension/adapters/next-request.js:91:23)
kruaco20-frontend-3  |     at NextRequestAdapter.fromBaseNextRequest (/app/node_modules/next/dist/server/web/spec-extension/adapters/next-request.js:70:35)
kruaco20-frontend-3  |     at doRender (/app/node_modules/next/dist/server/base-server.js:1330:73)
kruaco20-frontend-3  |     at cacheEntry.responseCache.get.routeKind (/app/node_modules/next/dist/server/base-server.js:1567:34)
kruaco20-frontend-3  |     at ResponseCache.get (/app/node_modules/next/dist/server/response-cache/index.js:49:26)
kruaco20-frontend-3  |     at NextNodeServer.renderToResponseWithComponentsImpl (/app/node_modules/next/dist/server/base-server.js:1475:53)
kruaco20-frontend-3  |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
kruaco20-frontend-3  |   code: 'ERR_INVALID_URL',
kruaco20-frontend-3  |   input: '/sitemap/test',
kruaco20-frontend-3  |   base: 'https, https://localhost:3000/sitemap/test'
kruaco20-frontend-3  | }

Expected result

notice that base become https, https://localhost:3000/sitemap/test which is incorrect. it should be without https, https://localhost:3000/sitemap/test

Verify canary release

  • [X] I verified that the issue exists in the latest Next.js canary release

Provide environment information

App is deployed using docker compose


Operating System:
  Platform: linux
  Arch: x64
  Version: #52~20.04.1-Ubuntu SMP Wed Sep 20 16:25:19 UTC 2023
Binaries:
  Node: 20.9.0
  npm: 10.1.0
  Yarn: 1.22.19
  pnpm: N/A
Relevant Packages:
  next: 14.0.3
  eslint-config-next: 14.0.3
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 4.8.3
Next.js Config:
  output: N/A

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

App Router

Additional context

In localhost both dev and production build works fine

intentionally call

new URL('/sitemap/test', 'https, https://localhost:3000/sitemap/test')

yields the same error

while

new URL('/sitemap/test', 'https://localhost:3000/sitemap/test')

works fine

Lodimup avatar Nov 26 '23 07:11 Lodimup

Theres a bug since 14.0.2 where the initUrl string will get multiple protocols when you have multiple x-forwarded-proto headers, sounds like you might be running in to that issue? I have made a PR for that issue.

Jonas-Blomqvist avatar Nov 27 '23 07:11 Jonas-Blomqvist

Theres a bug since 14.0.2 where the initUrl string will get multiple protocols when you have multiple x-forwarded-proto headers, sounds like you might be running in to that issue?

I have made a PR for that issue.

https://github.com/vercel/next.js/issues/58295#issuecomment-1826738095

This nginx workaround resolves the issue, but I don't think it's a proper fix. It may give you more insights into the issue.

We shouldn't have to put nginx in front to just fix this.

Lodimup avatar Nov 27 '23 08:11 Lodimup

Theres a bug since 14.0.2 where the initUrl string will get multiple protocols when you have multiple x-forwarded-proto headers, sounds like you might be running in to that issue? I have made a PR for that issue.

#58295 (comment)

This nginx workaround resolves the issue, but I don't think it's a proper fix. It may give you more insights into the issue.

We shouldn't have to put nginx in front to just fix this.

The nginx workaround "fixes" the issue because nginx strips the duplicate headers.

The PR I linked should fix the underlying issue.

Jonas-Blomqvist avatar Nov 27 '23 09:11 Jonas-Blomqvist

Any movement on this issue? this still seems to be kicking about still seeing the 'https' added when I am assuming it shouldn't be there:

  code: 'ERR_INVALID_URL',
  input: '/api/proxy/?shop=wandering-wombat',
  base: 'https, https://localhost:3000/api/proxy/?shop=wandering-wombat'

Confirming:

  • This is only an issue in development.
  • Built works as expected.

Originating here: const base = (0, _requestmeta.getRequestMeta)(request, "initURL") within: /node_modules/next/dist/server/web/spec-extension/adapters/next-request.js#L90

Not a fix just a workaround as working in production so to unblock me, it seems that it should do some further validation on the base.

                console.log('Temporary FIX ')
                console.log(base)
                const env = process.env.NODE_ENV

                if(env === "development") {
                    const newBase = base.split(',')[1] // strip the prefixed https
                    url = new URL(request.url, newBase);
                } else {
                    url = new URL(request.url, base);
                }
```
  
  

leighs-hammer avatar Mar 07 '24 15:03 leighs-hammer

Pardon me but I'm curious why didn't this bugfix (#58824) get backported to 14.1 while 14.2 haven't been out for 3 weeks. We have to stay in 14.0.1 or use the canary version.

zhyupe avatar Mar 22 '24 07:03 zhyupe

I used 14.1.2-canary.0 which #58824 was merged. But I can't find this commit in newer versions like 14.1.3 or 14.1.4 or even in 14.2.0-canary.X. I want to know when this commit will be merged into a stable version.

msdit avatar Mar 25 '24 14:03 msdit

FYI: We upgraded to the latest canary 14.2.0-canary.41 and it did fix the issue, so one might presume / hope it's in 14.2.0, although a back port would be nice.

nicholasgriffintn avatar Mar 25 '24 15:03 nicholasgriffintn

I can confirm that version 14.2.0 includes the fix.

rklos avatar May 09 '24 08:05 rklos

This still an issue on 14.2.0

Edit by maintainer bot: Comment was automatically minimized because it was considered unhelpful. (If you think this was by mistake, let us know). Please only comment if it adds context to the issue. If you want to express that you have the same problem, use the upvote 👍 on the issue description or subscribe to the issue for updates. Thanks!

younes200 avatar Aug 07 '24 11:08 younes200

I finally tracked down the source of this issue (aside from the multiple x-forwarded-proto issue that was resolved):

https://github.com/vercel/next.js/blob/3a39a205f7e8352b4c47b78a6f893eb7b67bdd28/packages/next/src/server/app-render/action-handler.ts#L479-L482

image

It's possible that the origin will be null. In my testing, I've seen that this happens if you have javascript disabled, or if you try to submit an action before the page is fully hydrated (e.g. try testing with cache disabled and a simulated 3g connection). MDN mentions that it could happen for various other reasons, and also links to a SO post with more detail (though I don't see anything about JS being disabled being a reason in either of those ¯\_(ツ)_/¯):

  • https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin#description
  • https://stackoverflow.com/questions/42239643/when-do-browsers-send-the-origin-header-when-do-browsers-set-the-origin-to-null/42242802

The code sees that the origin is a string and tries to parse it with new URL('null'), which leads to:

 ⨯ TypeError: Invalid URL
    at new URL (node:internal/url:797:36)
    at rP (/app/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:15:6487)
    at r9 (/app/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:18:1145)
    at /app/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:19:726
    at AsyncLocalStorage.run (node:async_hooks:346:14)
    at Object.wrap (/app/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:13:16657)
    at /app/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:19:616
    at AsyncLocalStorage.run (node:async_hooks:346:14)
    at Object.wrap (/app/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:13:15765)
    at r7 (/app/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:19:543) {
  code: 'ERR_INVALID_URL',
  input: 'null'
}

I'm testing with Next v14.2.4, but it looks like this would still be an issue in the canary branch.

A workaround is to add this to your middleware:

if (req.headers.get("origin") === "null") {
  req.headers.delete("origin");
}

You'll get this warning, but the action will otherwise work:

 ⚠ Missing `origin` header from a forwarded Server Actions request.

redbmk avatar Oct 23 '24 19:10 redbmk

Rereading through the MDN docs for the 5th time, I just realized that the reason I'm getting null actually has to do with me sending a referrer-policy: no-referrer header.

The Origin header value may be null in a number of cases, including (non-exhaustively):

  • ...
  • Referrer-Policy set to no-referrer for non-cors request modes (e.g. simple form posts).

It does make me wonder though, if one of the goals/features of server actions is progressive enhancement, and a simple form post is sent as a non-cors request, then should NextJS/React also send server actions in non-cors mode to match? At least in my case that would have caught this issue way earlier (as soon as we introduced all our security headers).

Ignoring null in the middleware or in the nextJS code as I suggested in my previous comment may actually be a security risk because it could potentially indicate a cross-origin request, so maybe something like referrer-policy: strict-origin would be a better value.

At least updating that code so that it doesn't just throw a TypeError (maybe wrapping in a try/catch) would be really helpful for debugging this for other devs with the same issue.

redbmk avatar Oct 24 '24 17:10 redbmk