Node: Changed behaviour for `instanceof ArrayBuffer`

[Zirro]

• Version: v10.2.1
• Platform: Darwin MacBook-Pro.local 17.6.0 Darwin Kernel Version 17.6.0: Tue May 8 15:33:09 PDT 2018; root:xnu-4570.61.1~2/RELEASE_X86_64 x86_64
• Subsystem: vm

Starting with the release of Node v10, we have seen a new behaviour with instanceof ArrayBuffer in jsdom. Our extensive use of the vm module is presumably involved.

I have extracted the following test case from the code in jsdom:

"use strict";

const vm = require("vm");

function Window() {
  this.console = console;

  this.ArrayBuffer = ArrayBuffer;
  this.Int8Array = Int8Array;
  this.Uint8Array = Uint8Array;
  this.Uint8ClampedArray = Uint8ClampedArray;
  this.Int16Array = Int16Array;
  this.Uint16Array = Uint16Array;
  this.Int32Array = Int32Array;
  this.Uint32Array = Uint32Array;
  this.Float32Array = Float32Array;
  this.Float64Array = Float64Array;
}

const window = new Window();

const script = new vm.Script(`
  const buffer = (new Uint8Array([0, 255, 0])).buffer;

  console.log(buffer instanceof ArrayBuffer);
`);

vm.createContext(window);

script.runInContext(window);

In Node v8.11.2, this logs true. In Node v10.2.1, it logs false.

I can't say for certain that the new behaviour is wrong - the assignments to this in Window were originally added to resolve issues with globals from different contexts, and removing them fixes this particular issue in Node v10. However, doing so reintroduces the previously mentioned issues as well.

[benjamingr]

This is because the ArrayBuffer passed from the outside is from a different "realm" than the one on the inside.

You can reproduce this error by passing any builtin for example if you pass Error then TypeError instanceof Error will evaluate to false.

> vm.runInNewContext('Error') instanceof Error; // false in REPL

This is correct behaviour - I'm not sure why Node.js 8 behaved differently.

[advanceddeveloper]

@benjamingr

TypeError is not an instance of Error, but TypeError() should be if both Error and TypeError are passed.

I'm not 100% sure, but it seems to me that @Zirro is right. I couldn't see why their script logs false. My guess is that in Node v8.11.2 accessing buffer of Uint8Array's instance constructs a new array buffer using global.ArrayBuffer constructor, but in Node v10.2.1 it probably calls some internal reference to ArrayBuffer which is in this case different from global.ArrayBuffer

[benjamingr]

@advanceddeveloper thank you for weighing in.

If both Error and TypeError were passed it would still not really catch these cases, namely if an _actual_ error was thrown it would get constructed with the "right" prototype and not the passed in one.

In general - I'm not sure what bug we had in Node.js 8 - but the v10 behavior is definitely more correct.

As a tangent - there are several proposals by TC39 to make this into a language (rather than Node.js) feature - https://github.com/tc39/proposal-realms

[targos]

My expectation is that if you call new Uint8Array() with the constructor from context A, its internal ArrayBuffer should also be from context A. (kind of like when you call an async function created in context A, it returns a Promise from context A).
I'm not certain but it looks like a V8 bug (in Node 10).

[targos]

btw if this is a bug, it is fixed in V8 6.7.

[Zirro]

Very interesting, thanks for looking into this. Going by the description, this commit seems relevant: https://github.com/v8/v8/commit/c68f863d7389f396b04f578a461c9fb386eb8535

[Zirro]

If we can confirm this as a bug per the above, could this potentially be fixed in a future 10.x release by backporting the relevant commits from V8?

[targos]

Can you check if it's fixed in Node 10.4.0? We upgraded V8 in this version

[Zirro]

Thanks, it has indeed been fixed in v10.4.0! I'll leave the issue open in case you want to add documentation about this as well, but otherwise you can close it.

[abonander]

@targos this is happening again as of Node v11.15.0; inside Jest I'm getting new Uint8Array().buffer instanceof ArrayBuffer === false. Unfortunately this check is in an indirect dependency so I can't work around it very easily.

[Hakerh400]

@abonander Unable to reproduce the issue on v11.15.0. Please note that this issue is related to the instanceof check only in case the ArrayBuffer constructor of the vm context is replaced with the ArrayBuffer contructor from the main context. If you hadn't explicitly passed ArrayBuffer constructor to the vm context and instanceof check failed, then it is working as intended. As I can see, authors of issues and PRs that referenced this issue are actually misunderstanding it.

Consider the following example:

const vm = require('vm');
const ctx = {
  console,
  buffer: new Uint8Array().buffer,
  f(){
    ctx.Uint8Array = Uint8Array;
    ctx.ArrayBuffer = ArrayBuffer;
  },
};
vm.createContext(ctx);
const script = new vm.Script(`
  console.log(new Uint8Array().buffer instanceof ArrayBuffer);
  console.log(buffer instanceof ArrayBuffer);
  f();
  console.log(new Uint8Array().buffer instanceof ArrayBuffer);
  console.log(buffer instanceof ArrayBuffer);
`);
script.runInContext(ctx);

Prior to v10.4.0 it printed

false
false
true

and after the issue is fixed it prints:

false
true
true

Note that it still prints false in the second output line, but it is not a bug.

[abonander]

This sounds like a Jest bug then because it doesn't give the user control over the VM context as far as I know.

[wi-ski]

This helped me: https://github.com/facebook/jest/issues/7780#issuecomment-615890410

[wi-ski]

This helped me: https://github.com/facebook/jest/issues/7780#issuecomment-615890410

[odahcam]

Happening in Node 12.14.1.

[silasdavis]

and v14.13.0