vm: use internal versions of compileFunction and Script

Instead of using the public versions of the vm APIs internally, use the internal versions so that we can skip unnecessary argument validation. The public versions would need special care to the generation of host-defined options to hit the isolate compilation cache when imporModuleDynamically isn't used, while internally it's almost always used, so this allows us to handle the host-defined options separately. PR-URL: nodejs#50137 Refs: nodejs#35375 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
joyeecheung · Dec 1, 2023 · 222877e · 222877e
1 parent c8be386
commit 222877e
Show file tree

Hide file tree

Showing 7 changed files with 245 additions and 173 deletions.
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -53,6 +53,7 @@ const {
   SafeMap,
   SafeWeakMap,
   String,
+  Symbol,
   StringPrototypeCharAt,
   StringPrototypeCharCodeAt,
   StringPrototypeEndsWith,
@@ -85,7 +86,12 @@ const {
   setOwnProperty,
   getLazy,
 } = require('internal/util');
-const { internalCompileFunction } = require('internal/vm');
+const {
+  internalCompileFunction,
+  makeContextifyScript,
+  runScriptInThisContext,
+} = require('internal/vm');
+
 const assert = require('internal/assert');
 const fs = require('fs');
 const path = require('path');
@@ -1236,7 +1242,6 @@ Module.prototype.require = function(id) {
 let resolvedArgv;
 let hasPausedEntry = false;
 /** @type {import('vm').Script} */
-let Script;
 
 /**
  * Wraps the given content in a script and runs it in a new context.
@@ -1245,46 +1250,49 @@ let Script;
  * @param {Module} cjsModuleInstance The CommonJS loader instance
  */
 function wrapSafe(filename, content, cjsModuleInstance) {
+  const hostDefinedOptionId = Symbol(`cjs:${filename}`);
+  async function importModuleDynamically(specifier, _, importAttributes) {
+    const cascadedLoader = getCascadedLoader();
+    return cascadedLoader.import(specifier, normalizeReferrerURL(filename),
+                                 importAttributes);
+  }
   if (patched) {
-    const wrapper = Module.wrap(content);
-    if (Script === undefined) {
-      ({ Script } = require('vm'));
-    }
-    const script = new Script(wrapper, {
-      filename,
-      lineOffset: 0,
-      importModuleDynamically: async (specifier, _, importAttributes) => {
-        const cascadedLoader = getCascadedLoader();
-        return cascadedLoader.import(specifier, normalizeReferrerURL(filename),
-                                     importAttributes);
-      },
-    });
+    const wrapped = Module.wrap(content);
+    const script = makeContextifyScript(
+      wrapped,                 // code
+      filename,                // filename
+      0,                       // lineOffset
+      0,                       // columnOffset
+      undefined,               // cachedData
+      false,                   // produceCachedData
+      undefined,               // parsingContext
+      hostDefinedOptionId,     // hostDefinedOptionId
+      importModuleDynamically, // importModuleDynamically
+    );
 
     // Cache the source map for the module if present.
     if (script.sourceMapURL) {
       maybeCacheSourceMap(filename, content, this, false, undefined, script.sourceMapURL);
     }
 
-    return script.runInThisContext({
-      displayErrors: true,
-    });
+    return runScriptInThisContext(script, true, false);
   }
 
+  const params = [ 'exports', 'require', 'module', '__filename', '__dirname' ];
   try {
-    const result = internalCompileFunction(content, [
-      'exports',
-      'require',
-      'module',
-      '__filename',
-      '__dirname',
-    ], {
-      filename,
-      importModuleDynamically(specifier, _, importAttributes) {
-        const cascadedLoader = getCascadedLoader();
-        return cascadedLoader.import(specifier, normalizeReferrerURL(filename),
-                                     importAttributes);
-      },
-    });
+    const result = internalCompileFunction(
+      content,                           // code,
+      filename,                          // filename
+      0,                                 // lineOffset
+      0,                                 // columnOffset,
+      undefined,                         // cachedData
+      false,                             // produceCachedData
+      undefined,                         // parsingContext
+      undefined,                         // contextExtensions
+      params,                            // params
+      hostDefinedOptionId,               // hostDefinedOptionId
+      importModuleDynamically,           // importModuleDynamically
+    );
 
     // Cache the source map for the module if present.
     if (result.sourceMapURL) {

diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js
@@ -1,6 +1,7 @@
 'use strict';
 
 const {
+  Symbol,
   RegExpPrototypeExec,
   globalThis,
 } = primordials;
@@ -24,7 +25,9 @@ const {
   emitAfter,
   popAsyncContext,
 } = require('internal/async_hooks');
-
+const {
+  makeContextifyScript, runScriptInThisContext,
+} = require('internal/vm');
 // shouldAbortOnUncaughtToggle is a typed array for faster
 // communication with JS.
 const { shouldAbortOnUncaughtToggle } = internalBinding('util');
@@ -52,8 +55,7 @@ function evalModule(source, print) {
 
 function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) {
   const CJSModule = require('internal/modules/cjs/loader').Module;
-  const { kVmBreakFirstLineSymbol } = require('internal/util');
-  const { pathToFileURL } = require('url');
+  const { pathToFileURL } = require('internal/url');
 
   const cwd = tryGetCwd();
   const origModule = globalThis.module;  // Set e.g. when called from the REPL.
@@ -78,16 +80,25 @@ function evalScript(name, body, breakFirstLine, print, shouldLoadESM = false) {
     `;
     globalThis.__filename = name;
     RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs.
-    const result = module._compile(script, `${name}-wrapper`)(() =>
-      require('vm').runInThisContext(body, {
-        filename: name,
-        displayErrors: true,
-        [kVmBreakFirstLineSymbol]: !!breakFirstLine,
-        importModuleDynamically(specifier, _, importAttributes) {
-          const loader = asyncESM.esmLoader;
-          return loader.import(specifier, baseUrl, importAttributes);
-        },
-      }));
+    const result = module._compile(script, `${name}-wrapper`)(() => {
+      const hostDefinedOptionId = Symbol(name);
+      async function importModuleDynamically(specifier, _, importAttributes) {
+        const loader = asyncESM.esmLoader;
+        return loader.import(specifier, baseUrl, importAttributes);
+      }
+      const script = makeContextifyScript(
+        body,                    // code
+        name,                    // filename,
+        0,                       // lineOffset
+        0,                       // columnOffset,
+        undefined,               // cachedData
+        false,                   // produceCachedData
+        undefined,               // parsingContext
+        hostDefinedOptionId,     // hostDefinedOptionId
+        importModuleDynamically, // importModuleDynamically
+      );
+      return runScriptInThisContext(script, true, !!breakFirstLine);
+    });
     if (print) {
       const { log } = require('internal/console/global');
       log(result);

diff --git a/lib/internal/vm.js b/lib/internal/vm.js
@@ -1,30 +1,25 @@
 'use strict';
 
 const {
-  ArrayPrototypeForEach,
+  ReflectApply,
   Symbol,
 } = primordials;
 
 const {
+  ContextifyScript,
   compileFunction,
   isContext: _isContext,
 } = internalBinding('contextify');
+const {
+  runInContext,
+} = ContextifyScript.prototype;
 const {
   default_host_defined_options,
 } = internalBinding('symbols');
 const {
-  validateArray,
-  validateBoolean,
-  validateBuffer,
   validateFunction,
   validateObject,
-  validateString,
-  validateStringArray,
-  validateInt32,
 } = require('internal/validators');
-const {
-  ERR_INVALID_ARG_TYPE,
-} = require('internal/errors').codes;
 
 function isContext(object) {
   validateObject(object, 'object', { __proto__: null, allowArray: true });
@@ -48,49 +43,20 @@ function getHostDefinedOptionId(importModuleDynamically, filename) {
   return Symbol(filename);
 }
 
-function internalCompileFunction(code, params, options) {
-  validateString(code, 'code');
-  if (params !== undefined) {
-    validateStringArray(params, 'params');
-  }
-  const {
-    filename = '',
-    columnOffset = 0,
-    lineOffset = 0,
-    cachedData = undefined,
-    produceCachedData = false,
-    parsingContext = undefined,
-    contextExtensions = [],
-    importModuleDynamically,
-  } = options;
-
-  validateString(filename, 'options.filename');
-  validateInt32(columnOffset, 'options.columnOffset');
-  validateInt32(lineOffset, 'options.lineOffset');
-  if (cachedData !== undefined)
-    validateBuffer(cachedData, 'options.cachedData');
-  validateBoolean(produceCachedData, 'options.produceCachedData');
-  if (parsingContext !== undefined) {
-    if (
-      typeof parsingContext !== 'object' ||
-      parsingContext === null ||
-      !isContext(parsingContext)
-    ) {
-      throw new ERR_INVALID_ARG_TYPE(
-        'options.parsingContext',
-        'Context',
-        parsingContext,
-      );
-    }
-  }
-  validateArray(contextExtensions, 'options.contextExtensions');
-  ArrayPrototypeForEach(contextExtensions, (extension, i) => {
-    const name = `options.contextExtensions[${i}]`;
-    validateObject(extension, name, { __proto__: null, nullable: true });
+function registerImportModuleDynamically(referrer, importModuleDynamically) {
+  const { importModuleDynamicallyWrap } = require('internal/vm/module');
+  const { registerModule } = require('internal/modules/esm/utils');
+  registerModule(referrer, {
+    __proto__: null,
+    importModuleDynamically:
+      importModuleDynamicallyWrap(importModuleDynamically),
   });
+}
 
-  const hostDefinedOptionId =
-      getHostDefinedOptionId(importModuleDynamically, filename);
+function internalCompileFunction(
+  code, filename, lineOffset, columnOffset,
+  cachedData, produceCachedData, parsingContext, contextExtensions,
+  params, hostDefinedOptionId, importModuleDynamically) {
   const result = compileFunction(
     code,
     filename,
@@ -117,23 +83,65 @@ function internalCompileFunction(code, params, options) {
   }
 
   if (importModuleDynamically !== undefined) {
-    validateFunction(importModuleDynamically,
-                     'options.importModuleDynamically');
-    const { importModuleDynamicallyWrap } = require('internal/vm/module');
-    const wrapped = importModuleDynamicallyWrap(importModuleDynamically);
-    const func = result.function;
-    const { registerModule } = require('internal/modules/esm/utils');
-    registerModule(func, {
-      __proto__: null,
-      importModuleDynamically: wrapped,
-    });
+    registerImportModuleDynamically(result.function, importModuleDynamically);
   }
 
   return result;
 }
 
+function makeContextifyScript(code,
+                              filename,
+                              lineOffset,
+                              columnOffset,
+                              cachedData,
+                              produceCachedData,
+                              parsingContext,
+                              hostDefinedOptionId,
+                              importModuleDynamically) {
+  let script;
+  // Calling `ReThrow()` on a native TryCatch does not generate a new
+  // abort-on-uncaught-exception check. A dummy try/catch in JS land
+  // protects against that.
+  try { // eslint-disable-line no-useless-catch
+    script = new ContextifyScript(code,
+                                  filename,
+                                  lineOffset,
+                                  columnOffset,
+                                  cachedData,
+                                  produceCachedData,
+                                  parsingContext,
+                                  hostDefinedOptionId);
+  } catch (e) {
+    throw e; /* node-do-not-add-exception-line */
+  }
+
+  if (importModuleDynamically !== undefined) {
+    registerImportModuleDynamically(script, importModuleDynamically);
+  }
+  return script;
+}
+
+// Internal version of vm.Script.prototype.runInThisContext() which skips
+// argument validation.
+function runScriptInThisContext(script, displayErrors, breakOnFirstLine) {
+  return ReflectApply(
+    runInContext,
+    script,
+    [
+      null,                // sandbox - use current context
+      -1,                  // timeout
+      displayErrors,       // displayErrors
+      false,               // breakOnSigint
+      breakOnFirstLine,    // breakOnFirstLine
+    ],
+  );
+}
+
 module.exports = {
   getHostDefinedOptionId,
   internalCompileFunction,
   isContext,
+  makeContextifyScript,
+  registerImportModuleDynamically,
+  runScriptInThisContext,
 };