Questions/clarifications about vm context behavior

[MSLaguana]

Node-chakracore currently has different behavior to node built on v8 for vm.runInContext, and we'd like to try to fix that. However to do so we'd first like to get a solid understanding of exactly how it is supposed to work.

My understanding is that vm.runInContext(script, sandbox) is supposed to run the given script as if the sandbox object were the global (but also have the normal global properties available). Any(?) changes made to the global object in that context are expected to be reflected back to the sandbox object, e.g. running “var x = 1” should cause the sandbox object to have an ‘x’ property with value 1, and “var y = this” should cause the sandbox object to have a ‘y’ property which references the sandbox object (not the global object in the context?). Is that both technically correct, and also the correct intuition for what the vm module is for?

If that’s right, then I have some follow up questions about the implementation as it stands today. I’m a little confused about how conflicts between properties are handled:

> var o = {NaN: 1, nan: 1};
> vm.createContext(o);
> vm.runInContext("NaN = 2; nan = 2;", o)
> o
{ NaN: 1, nan: 2 }

Why can we modify the non-shadowed property, but not the shadowed property? Is it because the global object’s NaN is non-writable, even though it is overridden by a writable property on the sandbox object?

> vm.runInContext("Object.defineProperty(this, 'bar', {value: 3, enumerable: true, configurable: true})", o)
{ NaN: 1, nan: 2, bar: 3 }
> vm.runInContext("Object.defineProperty(this, 'bar', {value: 4, enumerable: true, configurable: true})", o)
{ NaN: 1, nan: 2, bar: 3 }

Why can we define a property on the sandbox, but not redefine a property? Is this because it is non-writable, even though it should be the configurability which applies here?

> vm.runInContext("this.self = this", o)
{ self: [Circular] }
> o.self == o
false
> o
{ self: { self: [Circular] } }

Is it intentional that references to the actual-global can escape from the vm context and behave almost-but-not-quite like the sandbox object?

> o.NaN = 1
1
> vm.runInContext("NaN", o)
1
> vm.runInContext("delete NaN", o)
false
> vm.runInContext("NaN", o)
NaN

Is it intentional that if configurability differs between the sandbox and a global property, that delete operates partially, while if both are configurable then delete will remove the property from both the sandbox and the global?

[addaleax]

@nodejs/vm

[devsnek]

related heavily to #855, I'm actually working on a lot of this right now

[TimothyGu]

See #14660. I'd say that making these edge cases work in node-chakracore is not worth the effort, when we are not sure how node currently functions is how we want it to work in the first place.

[MSLaguana]

There are other cases that are currently broken in node-chakracore and are less edge-case-ish, such as expecting that vm.runInContext('var x = 1', o) will create an x property on o, so we definitely have some work to do in node-chakracore. We just want to make sure that we do the right work 😄

On that note, I see things like this:

> var o = {}
undefined
> vm.createContext(o)
{}
> vm.runInContext('var NaN = 1', o)
undefined
> o
{}
> o.NaN
undefined
> vm.runInContext('var nan = 2', o)
undefined
> o
{ nan: 2 }

so it looks like var foo = 1 only sometimes creates properties on the sandbox object, and silently does nothing if it does not?

[TimothyGu]

@MSLaguana We are explicitly ignoring those set operations. See

node/src/node_contextify.cc

Lines 338 to 346 in 78a7536

	bool is_declared_on_sandbox = ctx->sandbox()
	->GetRealNamedPropertyAttributes(ctx->context(), property)
	.To(&attributes);
	read_only = read_only ||
	(static_cast<int>(attributes) &
	static_cast<int>(PropertyAttribute::ReadOnly));
	if (read_only)
	return;

and surroundings.

[jasnell]

No further activity in over 2 years, closing