From f382139f9f9c7769396e64d07bc4bc9a45aa7fe6 Mon Sep 17 00:00:00 2001
From: Chengzhong Wu <chengzhong.wcz@alibaba-inc.com>
Date: Tue, 24 Oct 2023 16:18:01 +0800
Subject: [PATCH 1/2] vm: allow dynamic import with a referrer realm

A referrer can be a Script Record, a Cyclic Module Record, or a Realm
Record as defined in https://tc39.es/ecma262/#sec-HostLoadImportedModule.

Add support for dynamic import calls with a realm as the referrer and
allow specifying an `importModuleDynamically` callback in
`vm.createContext`.
---
 doc/api/vm.md                                 | 18 +++++
 lib/internal/modules/esm/utils.js             | 22 ++++--
 lib/internal/vm.js                            |  4 +-
 lib/vm.js                                     | 10 ++-
 src/module_wrap.cc                            | 22 +++---
 src/node_contextify.cc                        | 21 +++++-
 src/node_contextify.h                         |  1 +
 .../test-vm-module-referrer-realm.mjs         | 70 +++++++++++++++++++
 8 files changed, 146 insertions(+), 22 deletions(-)
 create mode 100644 test/parallel/test-vm-module-referrer-realm.mjs

diff --git a/doc/api/vm.md b/doc/api/vm.md
index 3978a14cbf7584..fd94c932db9de8 100644
--- a/doc/api/vm.md
+++ b/doc/api/vm.md
@@ -1052,6 +1052,9 @@ function with the given `params`.
 <!-- YAML
 added: v0.3.1
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/50360
+    description: The `importModuleDynamically` option is supported now.
   - version: v14.6.0
     pr-url: https://github.com/nodejs/node/pull/34023
     description: The `microtaskMode` option is supported now.
@@ -1084,6 +1087,21 @@ changes:
     scheduled through `Promise`s and `async function`s) will be run immediately
     after a script has run through [`script.runInContext()`][].
     They are included in the `timeout` and `breakOnSigint` scopes in that case.
+  * `importModuleDynamically` {Function} Called when `import()` is called in
+    this context without a referrer script or module. If this option is not
+    specified, calls to `import()` will reject with
+    [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. If
+    `--experimental-vm-modules` isn't set, this callback will be ignored and
+    calls to `import()` will reject with
+    [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`][].
+    * `specifier` {string} specifier passed to `import()`
+    * `contextObject` {Object} contextified object
+    * `importAttributes` {Object} The `"with"` value passed to the
+      [`optionsExpression`][] optional parameter, or an empty object if no value
+      was provided.
+    * Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
+      recommended in order to take advantage of error tracking, and to avoid
+      issues with namespaces that contain `then` function exports.
 * Returns: {Object} contextified object.
 
 If given a `contextObject`, the `vm.createContext()` method will [prepare
diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js
index 9d3e1aefbe3a31..3fe1daabe98f1d 100644
--- a/lib/internal/modules/esm/utils.js
+++ b/lib/internal/modules/esm/utils.js
@@ -113,6 +113,16 @@ function getConditionsSet(conditions) {
  */
 const moduleRegistries = new SafeWeakMap();
 
+/**
+ * @typedef {ContextifyScript|Function|ModuleWrap|ContextifiedObject} Referrer
+ * A referrer can be a Script Record, a Cyclic Module Record, or a Realm Record
+ * as defined in https://tc39.es/ecma262/#sec-HostLoadImportedModule.
+ *
+ * In Node.js, a referrer is represented by a wrapper object of these records.
+ * A referrer object has a field |host_defined_option_symbol| initialized with
+ * a symbol.
+ */
+
 /**
  * V8 would make sure that as long as import() can still be initiated from
  * the referrer, the symbol referenced by |host_defined_option_symbol| should
@@ -127,7 +137,7 @@ const moduleRegistries = new SafeWeakMap();
  * referrer wrap is still around and can be passed into the callbacks.
  * 2 is only there so that we can get the id symbol to configure the
  * weak map.
- * @param {ModuleWrap|ContextifyScript|Function} referrer The referrer to
+ * @param {Referrer} referrer The referrer to
  *   get the id symbol from. This is different from callbackReferrer which
  *   could be set by the caller.
  * @param {ModuleRegistry} registry
@@ -163,20 +173,20 @@ function initializeImportMetaObject(symbol, meta) {
 
 /**
  * Asynchronously imports a module dynamically using a callback function. The native callback.
- * @param {symbol} symbol - Reference to the module.
+ * @param {symbol} referrerSymbol - Referrer symbol of the registered script, function, module, or contextified object.
  * @param {string} specifier - The module specifier string.
  * @param {Record<string, string>} attributes - The import attributes object.
  * @returns {Promise<import('internal/modules/esm/loader.js').ModuleExports>} - The imported module object.
  * @throws {ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING} - If the callback function is missing.
  */
-async function importModuleDynamicallyCallback(symbol, specifier, attributes) {
-  if (moduleRegistries.has(symbol)) {
-    const { importModuleDynamically, callbackReferrer } = moduleRegistries.get(symbol);
+async function importModuleDynamicallyCallback(referrerSymbol, specifier, attributes) {
+  if (moduleRegistries.has(referrerSymbol)) {
+    const { importModuleDynamically, callbackReferrer } = moduleRegistries.get(referrerSymbol);
     if (importModuleDynamically !== undefined) {
       return importModuleDynamically(specifier, callbackReferrer, attributes);
     }
   }
-  if (symbol === vm_dynamic_import_missing_flag) {
+  if (referrerSymbol === vm_dynamic_import_missing_flag) {
     throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG();
   }
   throw new ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING();
diff --git a/lib/internal/vm.js b/lib/internal/vm.js
index b67eb177b35f07..6c7872fbf27b94 100644
--- a/lib/internal/vm.js
+++ b/lib/internal/vm.js
@@ -34,7 +34,7 @@ function isContext(object) {
   return _isContext(object);
 }
 
-function getHostDefinedOptionId(importModuleDynamically, filename) {
+function getHostDefinedOptionId(importModuleDynamically, hint) {
   if (importModuleDynamically !== undefined) {
     // Check that it's either undefined or a function before we pass
     // it into the native constructor.
@@ -57,7 +57,7 @@ function getHostDefinedOptionId(importModuleDynamically, filename) {
     return vm_dynamic_import_missing_flag;
   }
 
-  return Symbol(filename);
+  return Symbol(hint);
 }
 
 function registerImportModuleDynamically(referrer, importModuleDynamically) {
diff --git a/lib/vm.js b/lib/vm.js
index d396a3df8c0e13..44861be46dafd2 100644
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -218,6 +218,7 @@ function createContext(contextObject = {}, options = kEmptyObject) {
     origin,
     codeGeneration,
     microtaskMode,
+    importModuleDynamically,
   } = options;
 
   validateString(name, 'options.name');
@@ -239,7 +240,14 @@ function createContext(contextObject = {}, options = kEmptyObject) {
                 ['afterEvaluate', undefined]);
   const microtaskQueue = (microtaskMode === 'afterEvaluate');
 
-  makeContext(contextObject, name, origin, strings, wasm, microtaskQueue);
+  const hostDefinedOptionId =
+    getHostDefinedOptionId(importModuleDynamically, name);
+
+  makeContext(contextObject, name, origin, strings, wasm, microtaskQueue, hostDefinedOptionId);
+  // Register the context scope callback after the context was initialized.
+  if (importModuleDynamically !== undefined) {
+    registerImportModuleDynamically(contextObject, importModuleDynamically);
+  }
   return contextObject;
 }
 
diff --git a/src/module_wrap.cc b/src/module_wrap.cc
index 99802b372f432f..1d61fe5f0522ba 100644
--- a/src/module_wrap.cc
+++ b/src/module_wrap.cc
@@ -564,22 +564,20 @@ static MaybeLocal<Promise> ImportModuleDynamically(
 
   Local<Function> import_callback =
     env->host_import_module_dynamically_callback();
+  Local<Value> id;
 
   Local<FixedArray> options = host_defined_options.As<FixedArray>();
-  if (options->Length() != HostDefinedOptions::kLength) {
-    Local<Promise::Resolver> resolver;
-    if (!Promise::Resolver::New(context).ToLocal(&resolver)) return {};
-    resolver
-        ->Reject(context,
-                 v8::Exception::TypeError(FIXED_ONE_BYTE_STRING(
-                     context->GetIsolate(), "Invalid host defined options")))
-        .ToChecked();
-    return handle_scope.Escape(resolver->GetPromise());
+  // Get referrer id symbol from the host-defined options.
+  // If the host-defined options are empty, get the referrer id symbol
+  // from the realm global object.
+  if (options->Length() == HostDefinedOptions::kLength) {
+    id = options->Get(context, HostDefinedOptions::kID).As<Symbol>();
+  } else {
+    id = context->Global()
+             ->GetPrivate(context, env->host_defined_option_symbol())
+             .ToLocalChecked();
   }
 
-  Local<Symbol> id =
-      options->Get(context, HostDefinedOptions::kID).As<Symbol>();
-
   Local<Object> attributes =
       createImportAttributesContainer(env, isolate, import_attributes);
 
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index a849efcc9c334d..196d9d77f0f38e 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -288,6 +288,15 @@ BaseObjectPtr<ContextifyContext> ContextifyContext::New(
             .IsNothing()) {
       return BaseObjectPtr<ContextifyContext>();
     }
+
+    if (new_context_global
+            ->SetPrivate(v8_context,
+                         env->host_defined_option_symbol(),
+                         options->host_defined_options_id)
+            .IsNothing()) {
+      return BaseObjectPtr<ContextifyContext>();
+    }
+
     env->AssignToContext(v8_context, nullptr, info);
 
     if (!env->contextify_wrapper_template()
@@ -308,6 +317,13 @@ BaseObjectPtr<ContextifyContext> ContextifyContext::New(
           .IsNothing()) {
     return BaseObjectPtr<ContextifyContext>();
   }
+  if (sandbox_obj
+          ->SetPrivate(v8_context,
+                       env->host_defined_option_symbol(),
+                       options->host_defined_options_id)
+          .IsNothing()) {
+    return BaseObjectPtr<ContextifyContext>();
+  }
 
   return result;
 }
@@ -344,7 +360,7 @@ void ContextifyContext::RegisterExternalReferences(
 void ContextifyContext::MakeContext(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  CHECK_EQ(args.Length(), 6);
+  CHECK_EQ(args.Length(), 7);
   CHECK(args[0]->IsObject());
   Local<Object> sandbox = args[0].As<Object>();
 
@@ -375,6 +391,9 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo<Value>& args) {
         MicrotaskQueue::New(env->isolate(), MicrotasksPolicy::kExplicit);
   }
 
+  CHECK(args[6]->IsSymbol());
+  options.host_defined_options_id = args[6].As<Symbol>();
+
   TryCatchScope try_catch(env);
   BaseObjectPtr<ContextifyContext> context_ptr =
       ContextifyContext::New(env, sandbox, &options);
diff --git a/src/node_contextify.h b/src/node_contextify.h
index 721c146ff88c35..cce062622b70d8 100644
--- a/src/node_contextify.h
+++ b/src/node_contextify.h
@@ -18,6 +18,7 @@ struct ContextOptions {
   v8::Local<v8::Boolean> allow_code_gen_strings;
   v8::Local<v8::Boolean> allow_code_gen_wasm;
   std::unique_ptr<v8::MicrotaskQueue> own_microtask_queue;
+  v8::Local<v8::Symbol> host_defined_options_id;
 };
 
 class ContextifyContext : public BaseObject {
diff --git a/test/parallel/test-vm-module-referrer-realm.mjs b/test/parallel/test-vm-module-referrer-realm.mjs
new file mode 100644
index 00000000000000..3957f147d8ee99
--- /dev/null
+++ b/test/parallel/test-vm-module-referrer-realm.mjs
@@ -0,0 +1,70 @@
+// Flags: --experimental-vm-modules
+import * as common from '../common/index.mjs';
+import assert from 'node:assert';
+import { Script, SourceTextModule, createContext } from 'node:vm';
+
+async function test() {
+  const foo = new SourceTextModule('export const a = 1;');
+  await foo.link(common.mustNotCall());
+  await foo.evaluate();
+
+  const ctx = createContext({}, {
+    importModuleDynamically: common.mustCall((specifier, wrap) => {
+      assert.strictEqual(specifier, 'foo');
+      assert.strictEqual(wrap, ctx);
+      return foo;
+    }, 2),
+  });
+  {
+    const s = new Script('Promise.resolve("import(\'foo\')").then(eval)', {
+      importModuleDynamically: common.mustNotCall(),
+    });
+
+    const result = s.runInContext(ctx);
+    assert.strictEqual(await result, foo.namespace);
+  }
+
+  {
+    const m = new SourceTextModule('globalThis.fooResult = Promise.resolve("import(\'foo\')").then(eval)', {
+      context: ctx,
+      importModuleDynamically: common.mustNotCall(),
+    });
+    await m.link(common.mustNotCall());
+    await m.evaluate();
+    assert.strictEqual(await ctx.fooResult, foo.namespace);
+    delete ctx.fooResult;
+  }
+}
+
+async function testMissing() {
+  const ctx = createContext({});
+  {
+    const s = new Script('Promise.resolve("import(\'foo\')").then(eval)', {
+      importModuleDynamically: common.mustNotCall(),
+    });
+
+    const result = s.runInContext(ctx);
+    await assert.rejects(result, {
+      code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING',
+    });
+  }
+
+  {
+    const m = new SourceTextModule('globalThis.fooResult = Promise.resolve("import(\'foo\')").then(eval)', {
+      context: ctx,
+      importModuleDynamically: common.mustNotCall(),
+    });
+    await m.link(common.mustNotCall());
+    await m.evaluate();
+
+    await assert.rejects(ctx.fooResult, {
+      code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING',
+    });
+    delete ctx.fooResult;
+  }
+}
+
+await Promise.all([
+  test(),
+  testMissing(),
+]).then(common.mustCall());

From 5d6cadaa9aeec2c28d3e794e49c19929b2f4758b Mon Sep 17 00:00:00 2001
From: legendecas <legendecas@gmail.com>
Date: Wed, 25 Oct 2023 22:41:53 +0800
Subject: [PATCH 2/2] fixup! vm: allow dynamic import with a referrer realm

---
 src/node_contextify.cc                    | 7 +++++++
 test/es-module/test-esm-dynamic-import.js | 6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 196d9d77f0f38e..8b27b000f7da06 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -289,6 +289,10 @@ BaseObjectPtr<ContextifyContext> ContextifyContext::New(
       return BaseObjectPtr<ContextifyContext>();
     }
 
+    // Assign host_defined_options_id to the global object so that in the
+    // callback of ImportModuleDynamically, we can get the
+    // host_defined_options_id from the v8::Context without accessing the
+    // wrapper object.
     if (new_context_global
             ->SetPrivate(v8_context,
                          env->host_defined_option_symbol(),
@@ -317,6 +321,9 @@ BaseObjectPtr<ContextifyContext> ContextifyContext::New(
           .IsNothing()) {
     return BaseObjectPtr<ContextifyContext>();
   }
+  // Assign host_defined_options_id to the sandbox object so that module
+  // callbacks like importModuleDynamically can be registered once back to the
+  // JS land.
   if (sandbox_obj
           ->SetPrivate(v8_context,
                        env->host_defined_option_symbol(),
diff --git a/test/es-module/test-esm-dynamic-import.js b/test/es-module/test-esm-dynamic-import.js
index f69682ef3bca8f..9a6cfad18e1fb8 100644
--- a/test/es-module/test-esm-dynamic-import.js
+++ b/test/es-module/test-esm-dynamic-import.js
@@ -69,4 +69,10 @@ function expectFsNamespace(result) {
   // If the specifier is an origin-relative URL, it should
   // be treated as a file: URL.
   expectOkNamespace(import(targetURL.pathname));
+
+  // If the referrer is a realm record, there is no way to resolve the
+  // specifier.
+  // TODO(legendecas): https://github.com/tc39/ecma262/pull/3195
+  expectModuleError(Promise.resolve('import("node:fs")').then(eval),
+                    'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING');
 })();