patriksimek / vm2

Advanced vm/sandbox for Node.js
MIT License
3.87k stars 295 forks source link

Array converted to an array-like Object #437

Open Gilad-Gur-Andelman opened 2 years ago

Gilad-Gur-Andelman commented 2 years ago

Hi, I have this issue.

To reproduce, please run this code: (you can run it for example here)

const { VM } = require('vm2')
const assert = require('assert').strict;

const vm = new VM()
const res = vm.run('[1, 2, 3]')
assert.deepEqual(res,[1,2,3])

It yields this error:

AssertionError: Expected values to be strictly deep-equal:
+ actual - expected

  [
    1,
    2,
    3,
+   '0': 1,
+   '1': 2,
+   '2': 3
  ]

I believe this problem is closely related to this issue, where collaborator @XmiliaH wrote This is back for all versions as the fix broke Object.keys. As this is only a issue when logging objects this is acceptable.

However, as you can see, this is not only a logging issue.

Gilad-Gur-Andelman commented 2 years ago

Here are a couple more examples (resembles my own use case better) that yield the same error:

Example 1:

const {VM} = require("vm2")
const assert = require('assert').strict;
const vm = new VM(); // Objects specified in the sandbox cannot be frozen.
const a = [1,2,3]
vm.setGlobals({assert})
vm.freeze(a,'a');

vm.run('assert.deepEqual(a,[1,2,3])');

Example 2:

const {VM} = require("vm2")
const assert = require('assert').strict;
const vm = new VM(); // Objects specified in the sandbox cannot be frozen.
const a = [1,2,3]
vm.setGlobals({a,assert})

vm.run('assert.deepEqual(a,[1,2,3])');
Gilad-Gur-Andelman commented 2 years ago

198

XmiliaH commented 2 years ago

This is a node bug.

const assert = require('assert').strict;
const res = new Proxy([1,2,3],{
  ownKeys(target){
    return Reflect.ownKeys(target);
  }
});

assert.deepEqual(res,[1,2,3]);

When one removes the ownKeys hook everything works. There is nothing we can do on the vm2 side.

Gilad-Gur-Andelman commented 2 years ago

This is a node bug.

const assert = require('assert').strict;
const res = new Proxy([1,2,3],{
  ownKeys(target){
    return Reflect.ownKeys(target);
  }
});

assert.deepEqual(res,[1,2,3]);

When one removes the ownKeys hook everything works. There is nothing we can do on the vm2 side.

Thanks for the reply @XmiliaH. Attaching the nodejs issue for reference. Is there any way to work around this?

XmiliaH commented 2 years ago

Is there any way to work around this?

I tried to fix this issue already, but it broke other functionalities.

Gilad-Gur-Andelman commented 2 years ago

For those who are interested, I found a workaround which solves my use case.

The solution is to unproxify the returned object from vm.run (which is apparently wrapped in a proxy). How to do that? Well, simply deep copy/clone the object. I found a hint in this Stackoverflow thread. The top answer there suggests to JSON.parse(JSON.stringify(proxy)) in order to make it an object that is not wrapped in a proxy anymore.

For the sake of completeness, here are the examples above with the solution integrated in them, feel free to copy-paste them into here and check them out for yourself. Example 1:

const { VM } = require('vm2')
const assert = require('assert').strict;

const vm = new VM()
const res = vm.run('[1, 2, 3]')
assert.deepEqual(JSON.parse(JSON.stringify(res)),[1,2,3])

Example 2:

const {VM} = require("vm2")
const assert = require('assert').strict;
const vm = new VM(); // Objects specified in the sandbox cannot be frozen.
const a = [1,2,3]
vm.setGlobals({assert})
vm.freeze(a,'a');

vm.run('assert.deepEqual(JSON.parse(JSON.stringify(a)),[1,2,3])');

Example 3:

const {VM} = require("vm2")
const assert = require('assert').strict;
const vm = new VM();
const a = [1,2,3]
vm.setGlobals({a,assert})

vm.run('assert.deepEqual(JSON.parse(JSON.stringify(a)),[1,2,3])');

Note 1: Indeed, deep copying an object can be an expensive action, but for my use case it is an acceptable solution.

Note 2: Another solution is to cloneDeep with lodash. As suggested here, JSON.parse(JSON.stringify(proxy)) can be a destructive action and some properties of the new copy of the original object might be lost in the process (for example classes, functions and undefined properties). So in this solution the code will look exactly the same but need to import the cloneDeep function from lodash library and use it instead of JSON.parse(JSON.stringify(proxy)). lodash example:

const { VM } = require('vm2')
const assert = require('assert').strict;
const cloneDeep = require('lodash/cloneDeep')

const vm = new VM()
const res = vm.run('[undefined, null, 0, "s", true, [{a: 2}], {b: 3}]')
assert.deepEqual(cloneDeep(res),[undefined, null, 0, "s", true, [{a: 2}], {b: 3}])