vm: strict mode ReferenceError with assignment to context property

[bnoordhuis]

Split off from #5344 (comment).

'use strict';
const vm = require('vm');
const ctx = vm.createContext({ x: 42 });

try {
  const result = vm.runInContext('"use strict"; x = 1', ctx);
} catch(e) {
  console.log(e.stack);
  console.log('x is', ctx.x)
}

[bnoordhuis]

Probably cannot be fixed easily without regressing #10223. The problem is that GlobalPropertySetterCallback() does not check if the property exists on the context object.

Adding the check is not enough by itself. It should also record and check property attributes, otherwise non-writable properties can be overwritten.

[hashseed]

@fhinkel

[fhinkel]

We don't have coherent behavior what to do on the sandbox, when the setting/getting/deleting/defining fails or has side effects.

@AnnaMag is about to open a PR that removes the CopyProperties() hack. Once that landed we should come back to this and rethink it.

[Trott]

Should this remain open?

[fhinkel]

It's still an issue. We could add a known_issue test for it and close this.

[guo-yu]

I found the vm module still throws a weird error when replace 1 with a function like this:

'use strict';
const vm = require('vm');
const ctx = vm.createContext({ x: 42 });

try {
  const result = vm.runInContext('"use strict"; x = function demo(){};', ctx);
} catch(e) {
  console.log(e.stack);
  console.log('x is', ctx.x)
}

as it throw this error:

ReferenceError: x is not defined

Is there anything might be missing in the last bugfix patch?