vm: introduce vm.Context

Fixes nodejs#855
Fixes nodejs#31658
Fixes nodejs#31808

Author: devsnek

doc/api/vm.md

@@ -76,14 +76,13 @@ if (isMainThread) {
 }
 ```
 
-## `worker.moveMessagePortToContext(port, contextifiedSandbox)`
+## `worker.moveMessagePortToContext(port, context)`
 <!-- YAML
 added: v11.13.0
 -->
 
 * `port` {MessagePort} The message port which will be transferred.
-* `contextifiedSandbox` {Object} A [contextified][] object as returned by the
-  `vm.createContext()` method.
+* `context` {vm.Context} A [`vm.Context`][] instance.
 
 * Returns: {MessagePort}
 
@@ -768,6 +767,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
 [`trace_events`]: tracing.html
 [`v8.getHeapSnapshot()`]: v8.html#v8_v8_getheapsnapshot
 [`vm`]: vm.html
+[`vm.Context`]: vm.html#vm_class_vm_context
 [`worker.on('message')`]: #worker_threads_event_message_1
 [`worker.postMessage()`]: #worker_threads_worker_postmessage_value_transferlist
 [`worker.SHARE_ENV`]: #worker_threads_worker_share_env
@@ -780,5 +780,4 @@ active handle in the event system. If the worker is already `unref()`ed calling
 [Web Workers]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API
 [browser `MessagePort`]: https://developer.mozilla.org/en-US/docs/Web/API/MessagePort
 [child processes]: child_process.html
-[contextified]: vm.html#vm_what_does_it_mean_to_contextify_an_object
 [v8.serdes]: v8.html#v8_serialization_api

doc/api/worker_threads.md

@@ -47,7 +47,10 @@ const {
   validateBuffer,
   validateObject,
 } = require('internal/validators');
-const { kVmBreakFirstLineSymbol } = require('internal/util');
+const {
+  kVmBreakFirstLineSymbol,
+  emitExperimentalWarning,
+} = require('internal/util');
 const kParsingContext = Symbol('script parsing context');
 
 class Script extends ContextifyScript {
@@ -206,13 +209,11 @@ function isContext(object) {
 }
 
 let defaultContextNameIndex = 1;
-function createContext(contextObject = {}, options = {}) {
-  if (isContext(contextObject)) {
-    return contextObject;
+function validateContextOptions(options) {
+  if (typeof options !== 'object' || options === null) {
+    throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
   }
 
-  validateObject(options, 'options');
-
   const {
     name = `VM Context ${defaultContextNameIndex++}`,
     origin,
@@ -233,10 +234,35 @@ function createContext(contextObject = {}, options = {}) {
     validateBoolean(wasm, 'options.codeGeneration.wasm');
   }
 
-  makeContext(contextObject, name, origin, strings, wasm);
+  return { name, origin, strings, wasm };
+}
+
+function createContext(contextObject = {}, options = {}) {
+  if (isContext(contextObject)) {
+    return contextObject;
+  }
+
+  const { name, origin, strings, wasm } = validateContextOptions(options);
+
+  makeContext(contextObject, name, origin, strings, wasm, undefined);
   return contextObject;
 }
 
+class Context {
+  #global;
+
+  constructor(options = {}) {
+    emitExperimentalWarning('vm.Context');
+
+    const { name, origin, strings, wasm } = validateContextOptions(options);
+    this.#global = makeContext(undefined, name, origin, strings, wasm, this);
+  }
+
+  get global() {
+    return this.#global;
+  }
+}
+
 function createScript(code, options) {
   return new Script(code, options);
 }
@@ -357,6 +383,7 @@ function compileFunction(code, params, options = {}) {
 
 
 module.exports = {
+  Context,
   Script,
   createContext,
   createScript,

lib/vm.js

@@ -151,40 +151,44 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
     Local<Object> sandbox_obj,
     const ContextOptions& options) {
   EscapableHandleScope scope(env->isolate());
-  Local<FunctionTemplate> function_template =
-      FunctionTemplate::New(env->isolate());
-
-  function_template->SetClassName(sandbox_obj->GetConstructorName());
-
-  Local<ObjectTemplate> object_template =
-      function_template->InstanceTemplate();
 
-  Local<Object> data_wrapper;
-  if (!CreateDataWrapper(env).ToLocal(&data_wrapper))
-    return MaybeLocal<Context>();
-
-  NamedPropertyHandlerConfiguration config(
-      PropertyGetterCallback,
-      PropertySetterCallback,
-      PropertyDescriptorCallback,
-      PropertyDeleterCallback,
-      PropertyEnumeratorCallback,
-      PropertyDefinerCallback,
-      data_wrapper,
-      PropertyHandlerFlags::kHasNoSideEffect);
-
-  IndexedPropertyHandlerConfiguration indexed_config(
-      IndexedPropertyGetterCallback,
-      IndexedPropertySetterCallback,
-      IndexedPropertyDescriptorCallback,
-      IndexedPropertyDeleterCallback,
-      PropertyEnumeratorCallback,
-      IndexedPropertyDefinerCallback,
-      data_wrapper,
-      PropertyHandlerFlags::kHasNoSideEffect);
-
-  object_template->SetHandler(config);
-  object_template->SetHandler(indexed_config);
+  Local<ObjectTemplate> object_template;
+
+  if (!sandbox_obj.IsEmpty()) {
+    Local<FunctionTemplate> function_template =
+        FunctionTemplate::New(env->isolate());
+
+    function_template->SetClassName(sandbox_obj->GetConstructorName());
+
+    object_template = function_template->InstanceTemplate();
+
+    Local<Object> data_wrapper;
+    if (!CreateDataWrapper(env).ToLocal(&data_wrapper))
+      return MaybeLocal<Context>();
+
+    NamedPropertyHandlerConfiguration config(
+        PropertyGetterCallback,
+        PropertySetterCallback,
+        PropertyDescriptorCallback,
+        PropertyDeleterCallback,
+        PropertyEnumeratorCallback,
+        PropertyDefinerCallback,
+        data_wrapper,
+        PropertyHandlerFlags::kHasNoSideEffect);
+
+    IndexedPropertyHandlerConfiguration indexed_config(
+        IndexedPropertyGetterCallback,
+        IndexedPropertySetterCallback,
+        IndexedPropertyDescriptorCallback,
+        IndexedPropertyDeleterCallback,
+        PropertyEnumeratorCallback,
+        IndexedPropertyDefinerCallback,
+        data_wrapper,
+        PropertyHandlerFlags::kHasNoSideEffect);
+
+    object_template->SetHandler(config);
+    object_template->SetHandler(indexed_config);
+  }
 
   Local<Context> ctx = NewContext(env->isolate(), object_template);
 
@@ -194,16 +198,18 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
 
   ctx->SetSecurityToken(env->context()->GetSecurityToken());
 
-  // We need to tie the lifetime of the sandbox object with the lifetime of
-  // newly created context. We do this by making them hold references to each
-  // other. The context can directly hold a reference to the sandbox as an
-  // embedder data field. However, we cannot hold a reference to a v8::Context
-  // directly in an Object, we instead hold onto the new context's global
-  // object instead (which then has a reference to the context).
-  ctx->SetEmbedderData(ContextEmbedderIndex::kSandboxObject, sandbox_obj);
-  sandbox_obj->SetPrivate(env->context(),
-                          env->contextify_global_private_symbol(),
-                          ctx->Global());
+  if (!sandbox_obj.IsEmpty()) {
+    // We need to tie the lifetime of the sandbox object with the lifetime of
+    // newly created context. We do this by making them hold references to each
+    // other. The context can directly hold a reference to the sandbox as an
+    // embedder data field. However, we cannot hold a reference to a v8::Context
+    // directly in an Object, we instead hold onto the new context's global
+    // object instead (which then has a reference to the context).
+    ctx->SetEmbedderData(ContextEmbedderIndex::kSandboxObject, sandbox_obj);
+    sandbox_obj->SetPrivate(env->context(),
+                            env->contextify_global_private_symbol(),
+                            ctx->Global());
+  }
 
   Utf8Value name_val(env->isolate(), options.name);
   ctx->AllowCodeGenerationFromStrings(options.allow_code_gen_strings->IsTrue());
@@ -236,19 +242,23 @@ void ContextifyContext::Init(Environment* env, Local<Object> target) {
 }
 
 
-// makeContext(sandbox, name, origin, strings, wasm);
+// makeContext(sandbox, name, origin, strings, wasm, instance);
 void ContextifyContext::MakeContext(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  CHECK_EQ(args.Length(), 5);
-  CHECK(args[0]->IsObject());
-  Local<Object> sandbox = args[0].As<Object>();
+  CHECK_EQ(args.Length(), 6);
 
-  // Don't allow contextifying a sandbox multiple times.
-  CHECK(
-      !sandbox->HasPrivate(
-          env->context(),
-          env->contextify_context_private_symbol()).FromJust());
+  Local<Object> sandbox;
+  if (args[5]->IsUndefined()) {
+    CHECK(args[0]->IsObject());
+    sandbox = args[0].As<Object>();
+
+    // Don't allow contextifying a sandbox multiple times.
+    CHECK(
+        !sandbox->HasPrivate(
+            env->context(),
+            env->contextify_context_private_symbol()).FromJust());
+  }
 
   ContextOptions options;
 
@@ -278,7 +288,15 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo<Value>& args) {
   if (context_ptr->context().IsEmpty())
     return;
 
-  sandbox->SetPrivate(
+  Local<Object> instance;
+  if (args[5]->IsUndefined()) {
+    instance = sandbox;
+  } else {
+    CHECK(args[5]->IsObject());
+    args.GetReturnValue().Set(context_ptr->global_proxy());
+    instance = args[5].As<Object>();
+  }
+  instance->SetPrivate(
       env->context(),
       env->contextify_context_private_symbol(),
       External::New(env->isolate(), context_ptr.release()));

src/node_contextify.cc

@@ -0,0 +1,42 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { Context, Script } = require('vm');
+
+[
+  true, 0, null, '', 'hi',
+  Symbol('symbol'),
+  { name: 0 },
+  { origin: 0 },
+  { codeGeneration: 0 },
+  { codeGeneration: { strings: 0 } },
+  { codeGeneration: { wasm: 0 } },
+].forEach((v) => {
+  assert.throws(() => {
+    new Context(v);
+  }, {
+    code: 'ERR_INVALID_ARG_TYPE',
+  });
+});
+
+{
+  const ctx = new Context();
+  ctx.global.a = 1;
+  const script = new Script('this');
+  assert.strictEqual(script.runInContext(ctx), ctx.global);
+}
+
+// https://github.com/nodejs/node/issues/31808
+{
+  const ctx = new Context();
+  Object.defineProperty(ctx.global, 'x', {
+    enumerable: true,
+    configurable: true,
+    get: common.mustNotCall(),
+    set: common.mustNotCall(),
+  });
+  const script = new Script('function x() {}');
+  script.runInContext(ctx);
+  assert.strictEqual(typeof ctx.global.x, 'function');
+}

test/parallel/test-new-vm-context.js

@@ -84,7 +84,7 @@ const vm = require('vm');
 {
   const script = 'throw new Error("boom")';
   const filename = 'test-boom-error';
-  const context = vm.createContext();
+  const context = new vm.Context();
 
   function checkErr(err) {
     return err.stack.startsWith('test-boom-error:1');

test/parallel/test-vm-basic.js

@@ -3,13 +3,14 @@
 require('../common');
 const assert = require('assert');
 
-const { createContext, runInContext, runInNewContext } = require('vm');
+const { Context, runInContext, runInNewContext } = require('vm');
 
 const WASM_BYTES = Buffer.from(
   [0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00]);
 
 {
-  const ctx = createContext({ WASM_BYTES });
+  const ctx = new Context();
+  ctx.global.WASM_BYTES = WASM_BYTES;
   const test = 'eval(""); new WebAssembly.Module(WASM_BYTES);';
   runInContext(test, ctx);
 
@@ -19,7 +20,7 @@ const WASM_BYTES = Buffer.from(
 }
 
 {
-  const ctx = createContext({}, {
+  const ctx = new Context({
     codeGeneration: {
       strings: false,
     },
@@ -32,11 +33,12 @@ const WASM_BYTES = Buffer.from(
 }
 
 {
-  const ctx = createContext({ WASM_BYTES }, {
+  const ctx = new Context({
     codeGeneration: {
       wasm: false,
     },
   });
+  ctx.global.WASM_BYTES = WASM_BYTES;
 
   const CompileError = runInContext('WebAssembly.CompileError', ctx);
   assert.throws(() => {
@@ -65,7 +67,7 @@ assert.throws(() => {
 });
 
 assert.throws(() => {
-  createContext({}, {
+  new Context({
     codeGeneration: {
       strings: 0,
     },
@@ -85,15 +87,15 @@ assert.throws(() => {
 });
 
 assert.throws(() => {
-  createContext({}, {
+  new Context({
     codeGeneration: 1,
   });
 }, {
   code: 'ERR_INVALID_ARG_TYPE',
 });
 
 assert.throws(() => {
-  createContext({}, {
+  new Context({
     codeGeneration: null,
   });
 }, {