Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task done
Lodimup opened this issue Nov 26, 2023 · 11 comments
Open
1 task done
Labels
bug Issue was opened via the bug report template.

Comments

@Lodimup
Copy link

Lodimup commented Nov 26, 2023

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

  • 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 Lodimup added the bug Issue was opened via the bug report template. label Nov 26, 2023
@Jonas-PFX
Copy link
Contributor

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.

@Lodimup
Copy link
Author

Lodimup commented Nov 27, 2023

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.

@Jonas-PFX
Copy link
Contributor

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.

@leighs-hammer
Copy link

leighs-hammer commented Mar 7, 2024

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);
                }
```
  
  

@zhyupe
Copy link

zhyupe commented Mar 22, 2024

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.

@msdit
Copy link

msdit commented Mar 25, 2024

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.

@nicholasgriffintn
Copy link

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.

@rklos
Copy link

rklos commented May 9, 2024

I can confirm that version 14.2.0 includes the fix.

@younes200

This comment has been minimized.

@redbmk
Copy link
Contributor

redbmk commented Oct 23, 2024

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

const originDomain =
typeof req.headers['origin'] === 'string'
? new URL(req.headers['origin']).host
: undefined

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 ¯\_(ツ)_/¯):

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
Copy link
Contributor

redbmk commented Oct 24, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

No branches or pull requests

9 participants