diff --git a/doc/api/errors.md b/doc/api/errors.md index 6051437436a551..81074693f40020 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2981,6 +2981,12 @@ An attempt was made to use something that was already closed. While using the Performance Timing API (`perf_hooks`), no valid performance entry types are found. + + +### `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG` + +A dynamic import callback was invoked without `--experimental-vm-modules`. + ### `ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING` diff --git a/doc/api/vm.md b/doc/api/vm.md index e5c116d54af0bf..2043a39f527249 100644 --- a/doc/api/vm.md +++ b/doc/api/vm.md @@ -98,7 +98,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API. We do not recommend - using it in a production environment. + using it in a production environment. 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()` * `script` {vm.Script} * `importAssertions` {Object} The `"assert"` value passed to the @@ -760,6 +762,9 @@ changes: * `importModuleDynamically` {Function} Called during evaluation of this module when `import()` is called. 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()` * `module` {vm.Module} * `importAssertions` {Object} The `"assert"` value passed to the @@ -1018,7 +1023,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API, and should not be - considered stable. + considered stable. 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()` * `function` {Function} * `importAssertions` {Object} The `"assert"` value passed to the @@ -1242,7 +1249,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API. We do not recommend - using it in a production environment. + using it in a production environment. 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()` * `script` {vm.Script} * `importAssertions` {Object} The `"assert"` value passed to the @@ -1341,7 +1350,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API. We do not recommend - using it in a production environment. + using it in a production environment. 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()` * `script` {vm.Script} * `importAssertions` {Object} The `"assert"` value passed to the @@ -1421,7 +1432,9 @@ changes: when `import()` is called. If this option is not specified, calls to `import()` will reject with [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`][]. This option is part of the experimental modules API. We do not recommend - using it in a production environment. + using it in a production environment. 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()` * `script` {vm.Script} * `importAssertions` {Object} The `"assert"` value passed to the @@ -1585,6 +1598,7 @@ are not controllable through the timeout either. [Source Text Module Record]: https://tc39.es/ecma262/#sec-source-text-module-records [Synthetic Module Record]: https://heycam.github.io/webidl/#synthetic-module-records [V8 Embedder's Guide]: https://v8.dev/docs/embed#contexts +[`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG`]: errors.md#err_vm_dynamic_import_callback_missing_flag [`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`]: errors.md#err_vm_dynamic_import_callback_missing [`ERR_VM_MODULE_STATUS`]: errors.md#err_vm_module_status [`Error`]: errors.md#class-error diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 76ce7867043c7d..a72236063b47fd 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1821,6 +1821,9 @@ E('ERR_VALID_PERFORMANCE_ENTRY_TYPE', 'At least one valid performance entry type is required', Error); E('ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING', 'A dynamic import callback was not specified.', TypeError); +E('ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG', + 'A dynamic import callback was invoked without --experimental-vm-modules', + TypeError); E('ERR_VM_MODULE_ALREADY_LINKED', 'Module has already been linked', Error); E('ERR_VM_MODULE_CANNOT_CREATE_CACHED_DATA', 'Cached data cannot be created for a module which has been evaluated', Error); diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 8be147c8e233ca..b3b438372fe9ed 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -52,6 +52,7 @@ const { SafeMap, SafeWeakMap, String, + Symbol, StringPrototypeCharAt, StringPrototypeCharCodeAt, StringPrototypeEndsWith, @@ -84,7 +85,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'); @@ -1240,7 +1246,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. @@ -1250,47 +1255,49 @@ let Script; * @param {object} codeCache The SEA code cache */ function wrapSafe(filename, content, cjsModuleInstance, codeCache) { + 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, - cachedData: codeCache, - importModuleDynamically(specifier, _, importAttributes) { - const cascadedLoader = getCascadedLoader(); - return cascadedLoader.import(specifier, normalizeReferrerURL(filename), - importAttributes); - }, - }); + const result = internalCompileFunction( + content, // code, + filename, // filename + 0, // lineOffset + 0, // columnOffset, + codeCache, // cachedData + false, // produceCachedData + undefined, // parsingContext + undefined, // contextExtensions + params, // params + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); // The code cache is used for SEAs only. if (codeCache && diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 5627d98cf294d0..c36fe07a9503ae 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -15,6 +15,7 @@ const { StringPrototypeReplaceAll, StringPrototypeSlice, StringPrototypeStartsWith, + Symbol, SyntaxErrorPrototype, globalThis: { WebAssembly }, } = primordials; @@ -192,19 +193,29 @@ function enrichCJSError(err, content, filename) { */ function loadCJSModule(module, source, url, filename) { let compiledWrapper; + async function importModuleDynamically(specifier, _, importAttributes) { + return asyncESM.esmLoader.import(specifier, url, importAttributes); + } try { - compiledWrapper = internalCompileFunction(source, [ - 'exports', - 'require', - 'module', - '__filename', - '__dirname', - ], { - filename, - importModuleDynamically(specifier, _, importAttributes) { - return asyncESM.esmLoader.import(specifier, url, importAttributes); - }, - }).function; + compiledWrapper = internalCompileFunction( + source, // code, + filename, // filename + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + undefined, // contextExtensions + [ // params + 'exports', + 'require', + 'module', + '__filename', + '__dirname', + ], + Symbol(`cjs:${filename}`), // hostDefinedOptionsId + importModuleDynamically, // importModuleDynamically + ).function; } catch (err) { enrichCJSError(err, source, url); throw err; diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js index cbb583ede526b5..9d3e1aefbe3a31 100644 --- a/lib/internal/modules/esm/utils.js +++ b/lib/internal/modules/esm/utils.js @@ -14,9 +14,11 @@ const { } = internalBinding('util'); const { default_host_defined_options, + vm_dynamic_import_missing_flag, } = internalBinding('symbols'); const { + ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG, ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING, ERR_INVALID_ARG_VALUE, } = require('internal/errors').codes; @@ -132,7 +134,8 @@ const moduleRegistries = new SafeWeakMap(); */ function registerModule(referrer, registry) { const idSymbol = referrer[host_defined_option_symbol]; - if (idSymbol === default_host_defined_options) { + if (idSymbol === default_host_defined_options || + idSymbol === vm_dynamic_import_missing_flag) { // The referrer is compiled without custom callbacks, so there is // no registry to hold on to. We'll throw // ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING when a callback is @@ -173,6 +176,9 @@ async function importModuleDynamicallyCallback(symbol, specifier, attributes) { return importModuleDynamically(specifier, callbackReferrer, attributes); } } + if (symbol === 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/process/execution.js b/lib/internal/process/execution.js index 8ae6a1678af1b5..b8c507c798182e 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -1,6 +1,7 @@ 'use strict'; const { + Symbol, RegExpPrototypeExec, globalThis, } = primordials; @@ -25,7 +26,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'); @@ -53,7 +56,6 @@ 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('internal/url'); const cwd = tryGetCwd(); @@ -79,16 +81,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 index f348ef6d2d612f..b67eb177b35f07 100644 --- a/lib/internal/vm.js +++ b/lib/internal/vm.js @@ -1,28 +1,32 @@ 'use strict'; const { - ArrayPrototypeForEach, + ReflectApply, + Symbol, } = primordials; const { + ContextifyScript, compileFunction, isContext: _isContext, } = internalBinding('contextify'); const { - validateArray, - validateBoolean, - validateBuffer, + runInContext, +} = ContextifyScript.prototype; +const { + default_host_defined_options, + vm_dynamic_import_missing_flag, +} = internalBinding('symbols'); +const { validateFunction, validateObject, - validateString, - validateStringArray, kValidateObjectAllowArray, - kValidateObjectAllowNullable, - validateInt32, } = require('internal/validators'); + const { - ERR_INVALID_ARG_TYPE, -} = require('internal/errors').codes; + getOptionValue, +} = require('internal/options'); + function isContext(object) { validateObject(object, 'object', kValidateObjectAllowArray); @@ -30,48 +34,46 @@ function isContext(object) { return _isContext(object); } -function internalCompileFunction(code, params, options) { - validateString(code, 'code'); - if (params !== undefined) { - validateStringArray(params, 'params'); +function getHostDefinedOptionId(importModuleDynamically, filename) { + if (importModuleDynamically !== undefined) { + // Check that it's either undefined or a function before we pass + // it into the native constructor. + validateFunction(importModuleDynamically, + 'options.importModuleDynamically'); } - - 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, - ); - } + if (importModuleDynamically === undefined) { + // We need a default host defined options that are the same for all + // scripts not needing custom module callbacks so that the isolate + // compilation cache can be hit. + return default_host_defined_options; + } + // We should've thrown here immediately when we introduced + // --experimental-vm-modules and importModuleDynamically, but since + // users are already using this callback to throw a similar error, + // we also defer the error to the time when an actual import() is called + // to avoid breaking them. To ensure that the isolate compilation + // cache can still be hit, use a constant sentinel symbol here. + if (!getOptionValue('--experimental-vm-modules')) { + return vm_dynamic_import_missing_flag; } - validateArray(contextExtensions, 'options.contextExtensions'); - ArrayPrototypeForEach(contextExtensions, (extension, i) => { - const name = `options.contextExtensions[${i}]`; - validateObject(extension, name, kValidateObjectAllowNullable); + + return Symbol(filename); +} + +function registerImportModuleDynamically(referrer, importModuleDynamically) { + const { importModuleDynamicallyWrap } = require('internal/vm/module'); + const { registerModule } = require('internal/modules/esm/utils'); + registerModule(referrer, { + __proto__: null, + importModuleDynamically: + importModuleDynamicallyWrap(importModuleDynamically), }); +} +function internalCompileFunction( + code, filename, lineOffset, columnOffset, + cachedData, produceCachedData, parsingContext, contextExtensions, + params, hostDefinedOptionId, importModuleDynamically) { const result = compileFunction( code, filename, @@ -82,6 +84,7 @@ function internalCompileFunction(code, params, options) { parsingContext, contextExtensions, params, + hostDefinedOptionId, ); if (produceCachedData) { @@ -97,22 +100,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, }; diff --git a/lib/repl.js b/lib/repl.js index 52f3026414d72d..d8021215b3f425 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -118,6 +118,9 @@ const { } = require('internal/util'); const { inspect } = require('internal/util/inspect'); const vm = require('vm'); + +const { runInThisContext, runInContext } = vm.Script.prototype; + const path = require('path'); const fs = require('fs'); const { Interface } = require('readline'); @@ -185,7 +188,9 @@ const history = require('internal/repl/history'); const { extensionFormatMap, } = require('internal/modules/esm/formats'); - +const { + makeContextifyScript, +} = require('internal/vm'); let nextREPLResourceNumber = 1; // This prevents v8 code cache from getting confused and using a different // cache from a resource of the same name @@ -430,8 +435,6 @@ function REPLServer(prompt, } function defaultEval(code, context, file, cb) { - const asyncESM = require('internal/process/esm_loader'); - let result, script, wrappedErr; let err = null; let wrappedCmd = false; @@ -449,6 +452,21 @@ function REPLServer(prompt, wrappedCmd = true; } + const hostDefinedOptionId = Symbol(`eval:${file}`); + let parentURL; + try { + const { pathToFileURL } = require('internal/url'); + // Adding `/repl` prevents dynamic imports from loading relative + // to the parent of `process.cwd()`. + parentURL = pathToFileURL(path.join(process.cwd(), 'repl')).href; + } catch { + // Continue regardless of error. + } + async function importModuleDynamically(specifier, _, importAttributes) { + const asyncESM = require('internal/process/esm_loader'); + return asyncESM.esmLoader.import(specifier, parentURL, + importAttributes); + } // `experimentalREPLAwait` is set to true by default. // Shall be false in case `--no-experimental-repl-await` flag is used. if (experimentalREPLAwait && StringPrototypeIncludes(code, 'await')) { @@ -466,28 +484,21 @@ function REPLServer(prompt, } catch (e) { let recoverableError = false; if (e.name === 'SyntaxError') { - let parentURL; - try { - const { pathToFileURL } = require('internal/url'); - // Adding `/repl` prevents dynamic imports from loading relative - // to the parent of `process.cwd()`. - parentURL = pathToFileURL(path.join(process.cwd(), 'repl')).href; - } catch { - // Continue regardless of error. - } - // Remove all "await"s and attempt running the script // in order to detect if error is truly non recoverable const fallbackCode = SideEffectFreeRegExpPrototypeSymbolReplace(/\bawait\b/g, code, ''); try { - vm.createScript(fallbackCode, { - filename: file, - displayErrors: true, - importModuleDynamically: (specifier, _, importAttributes) => { - return asyncESM.esmLoader.import(specifier, parentURL, - importAttributes); - }, - }); + makeContextifyScript( + fallbackCode, // code + file, // filename, + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); } catch (fallbackError) { if (isRecoverableError(fallbackError, fallbackCode)) { recoverableError = true; @@ -507,15 +518,6 @@ function REPLServer(prompt, return cb(null); if (err === null) { - let parentURL; - try { - const { pathToFileURL } = require('internal/url'); - // Adding `/repl` prevents dynamic imports from loading relative - // to the parent of `process.cwd()`. - parentURL = pathToFileURL(path.join(process.cwd(), 'repl')).href; - } catch { - // Continue regardless of error. - } while (true) { try { if (self.replMode === module.exports.REPL_MODE_STRICT && @@ -524,14 +526,17 @@ function REPLServer(prompt, // value for statements and declarations that don't return a value. code = `'use strict'; void 0;\n${code}`; } - script = vm.createScript(code, { - filename: file, - displayErrors: true, - importModuleDynamically: (specifier, _, importAttributes) => { - return asyncESM.esmLoader.import(specifier, parentURL, - importAttributes); - }, - }); + script = makeContextifyScript( + code, // code + file, // filename, + 0, // lineOffset + 0, // columnOffset, + undefined, // cachedData + false, // produceCachedData + undefined, // parsingContext + hostDefinedOptionId, // hostDefinedOptionId + importModuleDynamically, // importModuleDynamically + ); } catch (e) { debug('parse error %j', code, e); if (wrappedCmd) { @@ -591,9 +596,9 @@ function REPLServer(prompt, }; if (self.useGlobal) { - result = script.runInThisContext(scriptOptions); + result = ReflectApply(runInThisContext, script, [scriptOptions]); } else { - result = script.runInContext(context, scriptOptions); + result = ReflectApply(runInContext, script, [context, scriptOptions]); } } finally { if (self.breakEvalOnSigint) { diff --git a/lib/vm.js b/lib/vm.js index f134cdc983db6d..d396a3df8c0e13 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -39,14 +39,16 @@ const { ERR_INVALID_ARG_TYPE, } = require('internal/errors').codes; const { + validateArray, validateBoolean, validateBuffer, - validateFunction, validateInt32, - validateObject, validateOneOf, + validateObject, validateString, + validateStringArray, validateUint32, + kValidateObjectAllowNullable, } = require('internal/validators'); const { emitExperimentalWarning, @@ -54,8 +56,10 @@ const { kVmBreakFirstLineSymbol, } = require('internal/util'); const { + getHostDefinedOptionId, internalCompileFunction, isContext, + registerImportModuleDynamically, } = require('internal/vm'); const kParsingContext = Symbol('script parsing context'); @@ -86,12 +90,8 @@ class Script extends ContextifyScript { } validateBoolean(produceCachedData, 'options.produceCachedData'); - if (importModuleDynamically !== undefined) { - // Check that it's either undefined or a function before we pass - // it into the native constructor. - validateFunction(importModuleDynamically, - 'options.importModuleDynamically'); - } + const hostDefinedOptionId = + getHostDefinedOptionId(importModuleDynamically, filename); // 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. @@ -103,19 +103,13 @@ class Script extends ContextifyScript { cachedData, produceCachedData, parsingContext, - importModuleDynamically !== undefined); + hostDefinedOptionId); } catch (e) { throw e; /* node-do-not-add-exception-line */ } if (importModuleDynamically !== undefined) { - const { importModuleDynamicallyWrap } = require('internal/vm/module'); - const { registerModule } = require('internal/modules/esm/utils'); - registerModule(this, { - __proto__: null, - importModuleDynamically: - importModuleDynamicallyWrap(importModuleDynamically), - }); + registerImportModuleDynamically(this, importModuleDynamically); } } @@ -302,7 +296,54 @@ function runInThisContext(code, options) { } function compileFunction(code, params, options = kEmptyObject) { - return internalCompileFunction(code, params, options).function; + 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, kValidateObjectAllowNullable); + }); + + const hostDefinedOptionId = + getHostDefinedOptionId(importModuleDynamically, filename); + + return internalCompileFunction( + code, filename, lineOffset, columnOffset, + cachedData, produceCachedData, parsingContext, contextExtensions, + params, hostDefinedOptionId, importModuleDynamically, + ).function; } const measureMemoryModes = { diff --git a/src/env_properties.h b/src/env_properties.h index 93fa8afdca39d3..7356846d6e7085 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -46,7 +46,8 @@ V(owner_symbol, "owner_symbol") \ V(onpskexchange_symbol, "onpskexchange") \ V(resource_symbol, "resource_symbol") \ - V(trigger_async_id_symbol, "trigger_async_id_symbol") + V(trigger_async_id_symbol, "trigger_async_id_symbol") \ + V(vm_dynamic_import_missing_flag, "vm_dynamic_import_missing_flag") // Strings are per-isolate primitives but Environment proxies them // for the sake of convenience. Strings should be ASCII-only. diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 466c2d81c7e7a9..4c7988eaed7c9e 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -771,11 +771,11 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { bool produce_cached_data = false; Local parsing_context = context; - bool needs_custom_host_defined_options = false; + Local id_symbol; if (argc > 2) { // new ContextifyScript(code, filename, lineOffset, columnOffset, // cachedData, produceCachedData, parsingContext, - // needsCustomHostDefinedOptions) + // hostDefinedOptionId) CHECK_EQ(argc, 8); CHECK(args[2]->IsNumber()); line_offset = args[2].As()->Value(); @@ -795,9 +795,8 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { CHECK_NOT_NULL(sandbox); parsing_context = sandbox->context(); } - if (args[7]->IsTrue()) { - needs_custom_host_defined_options = true; - } + CHECK(args[7]->IsSymbol()); + id_symbol = args[7].As(); } ContextifyScript* contextify_script = @@ -821,12 +820,6 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { Local host_defined_options = PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength); - // We need a default host defined options that's the same for all scripts - // not needing custom module callbacks for so that the isolate compilation - // cache can be hit. - Local id_symbol = needs_custom_host_defined_options - ? Symbol::New(isolate, filename) - : env->default_host_defined_options(); host_defined_options->Set( isolate, loader::HostDefinedOptions::kID, id_symbol); @@ -1199,6 +1192,10 @@ void ContextifyContext::CompileFunction( params_buf = args[8].As(); } + // Argument 10: host-defined option symbol + CHECK(args[9]->IsSymbol()); + Local id_symbol = args[9].As(); + // Read cache from cached data buffer ScriptCompiler::CachedData* cached_data = nullptr; if (!cached_data_buf.IsEmpty()) { @@ -1210,7 +1207,6 @@ void ContextifyContext::CompileFunction( // Set host_defined_options Local host_defined_options = PrimitiveArray::New(isolate, loader::HostDefinedOptions::kLength); - Local id_symbol = Symbol::New(isolate, filename); host_defined_options->Set( isolate, loader::HostDefinedOptions::kID, id_symbol); diff --git a/test/message/eval_messages.out b/test/message/eval_messages.out index 3f44332c03a470..e07bbe4d6acd3c 100644 --- a/test/message/eval_messages.out +++ b/test/message/eval_messages.out @@ -2,10 +2,9 @@ [eval]:1 with(this){__filename} ^^^^ + SyntaxError: Strict mode code may not include a with statement - at new Script (node:vm:*:*) - at createScript (node:vm:*:*) - at Object.runInThisContext (node:vm:*:*) + at makeContextifyScript (node:internal/vm:*:*) at node:internal/process/execution:*:* at [eval]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -21,8 +20,7 @@ throw new Error("hello") Error: hello at [eval]:1:7 - at Script.runInThisContext (node:vm:*:*) - at Object.runInThisContext (node:vm:*:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [eval]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -37,8 +35,7 @@ throw new Error("hello") Error: hello at [eval]:1:7 - at Script.runInThisContext (node:vm:*:*) - at Object.runInThisContext (node:vm:*:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [eval]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -53,8 +50,7 @@ var x = 100; y = x; ReferenceError: y is not defined at [eval]:1:16 - at Script.runInThisContext (node:vm:*:*) - at Object.runInThisContext (node:vm:*:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [eval]-wrapper:*:* at runScript (node:internal/process/execution:*:*) diff --git a/test/message/stdin_messages.out b/test/message/stdin_messages.out index 3fd047aacb11a2..6afc8a62d7fcd9 100644 --- a/test/message/stdin_messages.out +++ b/test/message/stdin_messages.out @@ -4,9 +4,7 @@ with(this){__filename} ^^^^ SyntaxError: Strict mode code may not include a with statement - at new Script (node:vm:*) - at createScript (node:vm:*) - at Object.runInThisContext (node:vm:*) + at makeContextifyScript (node:internal/vm:*:*) at node:internal/process/execution:*:* at [stdin]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -14,6 +12,8 @@ SyntaxError: Strict mode code may not include a with statement at node:internal/main/eval_stdin:*:* at Socket. (node:internal/process/execution:*:*) at Socket.emit (node:events:*:*) + at endReadableNT (node:internal/streams/readable:*:*) + at process.processTicksAndRejections (node:internal/process/task_queues:*:*) Node.js * 42 @@ -24,8 +24,7 @@ throw new Error("hello") Error: hello at [stdin]:1:7 - at Script.runInThisContext (node:vm:*) - at Object.runInThisContext (node:vm:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [stdin]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -33,6 +32,7 @@ Error: hello at node:internal/main/eval_stdin:*:* at Socket. (node:internal/process/execution:*:*) at Socket.emit (node:events:*:*) + at endReadableNT (node:internal/streams/readable:*:*) Node.js * [stdin]:1 @@ -41,8 +41,7 @@ throw new Error("hello") Error: hello at [stdin]:1:* - at Script.runInThisContext (node:vm:*) - at Object.runInThisContext (node:vm:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [stdin]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -50,6 +49,7 @@ Error: hello at node:internal/main/eval_stdin:*:* at Socket. (node:internal/process/execution:*:*) at Socket.emit (node:events:*:*) + at endReadableNT (node:internal/streams/readable:*:*) Node.js * 100 @@ -59,8 +59,7 @@ let x = 100; y = x; ReferenceError: y is not defined at [stdin]:1:16 - at Script.runInThisContext (node:vm:*) - at Object.runInThisContext (node:vm:*) + at runScriptInThisContext (node:internal/vm:*:*) at node:internal/process/execution:*:* at [stdin]-wrapper:*:* at runScript (node:internal/process/execution:*:*) @@ -68,6 +67,7 @@ ReferenceError: y is not defined at node:internal/main/eval_stdin:*:* at Socket. (node:internal/process/execution:*:*) at Socket.emit (node:events:*:*) + at endReadableNT (node:internal/streams/readable:*:*) Node.js * diff --git a/test/parallel/test-vm-dynamic-import-callback-missing-flag.js b/test/parallel/test-vm-dynamic-import-callback-missing-flag.js new file mode 100644 index 00000000000000..4b0d09ca3674a7 --- /dev/null +++ b/test/parallel/test-vm-dynamic-import-callback-missing-flag.js @@ -0,0 +1,28 @@ +'use strict'; + +const common = require('../common'); +const { Script, compileFunction } = require('vm'); +const assert = require('assert'); + +assert( + !process.execArgv.includes('--experimental-vm-modules'), + 'This test must be run without --experimental-vm-modules'); + +assert.rejects(async () => { + const script = new Script('import("fs")', { + importModuleDynamically: common.mustNotCall(), + }); + const imported = script.runInThisContext(); + await imported; +}, { + code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG' +}).then(common.mustCall()); + +assert.rejects(async () => { + const imported = compileFunction('return import("fs")', [], { + importModuleDynamically: common.mustNotCall(), + })(); + await imported; +}, { + code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING_FLAG' +}).then(common.mustCall()); diff --git a/test/parallel/test-vm-no-dynamic-import-callback.js b/test/parallel/test-vm-no-dynamic-import-callback.js index 3651f997598b21..35b553d587c6e4 100644 --- a/test/parallel/test-vm-no-dynamic-import-callback.js +++ b/test/parallel/test-vm-no-dynamic-import-callback.js @@ -1,7 +1,7 @@ 'use strict'; -require('../common'); -const { Script } = require('vm'); +const common = require('../common'); +const { Script, compileFunction } = require('vm'); const assert = require('assert'); assert.rejects(async () => { @@ -10,4 +10,11 @@ assert.rejects(async () => { await imported; }, { code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING' -}); +}).then(common.mustCall()); + +assert.rejects(async () => { + const imported = compileFunction('return import("fs")')(); + await imported; +}, { + code: 'ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING' +}).then(common.mustCall());