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

instanceof doesn't work in middleware v12.2 #38184

Closed
1 task done
meikidd opened this issue Jun 30, 2022 · 7 comments
Closed
1 task done

instanceof doesn't work in middleware v12.2 #38184

meikidd opened this issue Jun 30, 2022 · 7 comments
Labels
Runtime Related to Node.js or Edge Runtime with Next.js.

Comments

@meikidd
Copy link

meikidd commented Jun 30, 2022

Verify canary release

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

Provide environment information

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 21.2.0: Sun Nov 28 20:28:54 PST 2021; root:xnu-8019.61.5~1/RELEASE_X86_64
Binaries:
  Node: 17.8.0
  npm: 8.5.5
  Yarn: 1.22.18
  pnpm: N/A
Relevant packages:
  next: 12.2.0
  eslint-config-next: 12.2.0
  react: 18.2.0
  react-dom: 18.2.0

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

The instanceof operator doesn't work in the new released middleware.

export async function middleware(request: NextRequest) {
  const response = NextResponse.next();

  const obj = { hello: "world" };
  console.log(obj instanceof Object); // <-- this prints "false" in new middleware

  return response;
}

Expected Behavior

Expect above example to print "true".

Link to reproduction

https://github.com/meikidd/nextjs-middleware-instanceof

To Reproduce

  1. npm run dev
  2. Open "http://localhost:3000"
  3. Check the logs in Terminal. We expect "true", but actually get "false".
@meikidd meikidd added the bug Issue was opened via the bug report template. label Jun 30, 2022
@balazsorban44 balazsorban44 added Middleware Related to Next.js Middleware kind: bug Runtime Related to Node.js or Edge Runtime with Next.js. and removed bug Issue was opened via the bug report template. labels Jun 30, 2022
@feugy
Copy link
Member

feugy commented Jun 30, 2022

Thanks you @meikidd.
Indeed, it is an issue we have in dev only.

Calling instanceof on literals will fail because we're running the code in a sandbox that has different primitives as Node.js:

// will return false in the middleware
[] instanceof Array
{} instanceof Object
/^hello$/ instanceof RegExp

We're working on a fix, but it's a tricky problem, due to the nature of Node.js VM module.

@javivelasco javivelasco removed the Middleware Related to Next.js Middleware label Jun 30, 2022
@SukkaW
Copy link
Contributor

SukkaW commented Jul 1, 2022

@feugy FYI, there is a dirty hack for this:

Array = [].constructor;
Object = {}.constructor;
RegExp = /^/.constructor;

Just prepend this before the code that needs to be executed (instead of replacing the global objects with vm).

@feugy
Copy link
Member

feugy commented Jul 1, 2022

That is... fishy indeed 😅
But it'll be greatly useful. Let me run more testing on our end. Thanks @SukkaW !!

@SukkaW
Copy link
Contributor

SukkaW commented Jul 1, 2022

@feugy If the issue only occurs in dev mode, IMHO the hacky workaround should work.

Also, per Node.js' vm API Documentation:

The node:vm module is not a security mechanism. Do not use it to run untrusted code.

Although it is only used in local developing environment, it is still technically not safe, and also causes behavior mismatches between development and production (just like this issue).

But there is an escape hatch. There are ongoing ECMAScript proposals proposal-compartments (currently at stage 1) and proposal-shadowrealm (currently at stage 3 already!) to help developer creating a safe, secure sandbox for executing code!

@feugy
Copy link
Member

feugy commented Jul 1, 2022

Thanks for the pointers on the incoming ECMA proposals. As you said it, edge-runtime is currently for dev only, and is not meant to provide strong isolation. We'll surely revise is in the future, but it is out of scope for now.

We did some investigation with @javivelasco about the hack, and unfortunately it's not gonna work.

TLDR; it's fixing instanceof, but breaking the polyfills

The EdgeVM (node.js's VM module under the hood), has its own realm with its own Array constructor.
Since it's just a plain JavaScript environment, we must load some polyfills into it, to provide utilities such as TextEncoder, FormData and Fetch (there are dozens) of them.

In the current implementation, we load the polyfills in node.js realm, and injects them into the EdgeVM as globals.
But there's an drawback: the Array constructor used in the polyfill is the comming from node.js realm, while the EdgeVM has its own one.
To fix this inconsistency, we also inject node.js' Array into the EdgeVM context.

However, the JS engine does not use global Array constructor while building an array literal. And there's no way to change this, because it's how the ECMAscript spec defines it. So we have an inconsistency leading to this bug.

The suggested hack: Array = [].constructor reverts it back: the Array constructor inside EdgeVM realm becomes the same as the array literal, and different from the one used in the polyfills.


The only way to definitely fix this is to stop injecting anything from node.js, and load all the polyfills within the EdgeVM.
It is nowhere simple, because EdgeVM (node.js's VM module) is just a barebone JavaScript engine. It has no Buffer, no require()... And we need to provide or implement them.

@Kikobeats
Copy link
Member

Kikobeats commented Jul 22, 2022

Hello, this has been addressed and shipped at Next.js v12.2.3:

console.log(request instanceof Object) // true
console.log(response instanceof Object) // true
console.log({} instanceof Object) // true

You can see more examples Edge Runtime test suite 🙂

@github-actions
Copy link
Contributor

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Runtime Related to Node.js or Edge Runtime with Next.js.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants