diff --git a/lib/internal/vm.js b/lib/internal/vm.js index 70dbd866e66bd7..7ad76ebc873f19 100644 --- a/lib/internal/vm.js +++ b/lib/internal/vm.js @@ -2,12 +2,16 @@ const { ArrayPrototypeForEach, + Symbol, } = primordials; const { compileFunction, isContext: _isContext, } = internalBinding('contextify'); +const { + default_host_defined_options, +} = internalBinding('symbols'); const { validateArray, validateBoolean, @@ -28,12 +32,27 @@ function isContext(object) { return _isContext(object); } +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'); + } + 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; + } + return Symbol(filename); +} + function internalCompileFunction(code, params, options) { validateString(code, 'code'); if (params !== undefined) { validateStringArray(params, 'params'); } - const { filename = '', columnOffset = 0, @@ -70,6 +89,8 @@ function internalCompileFunction(code, params, options) { validateObject(extension, name, { __proto__: null, nullable: true }); }); + const hostDefinedOptionId = + getHostDefinedOptionId(importModuleDynamically, filename); const result = compileFunction( code, filename, @@ -80,6 +101,7 @@ function internalCompileFunction(code, params, options) { parsingContext, contextExtensions, params, + hostDefinedOptionId, ); if (produceCachedData) { @@ -111,6 +133,7 @@ function internalCompileFunction(code, params, options) { } module.exports = { + getHostDefinedOptionId, internalCompileFunction, isContext, }; diff --git a/lib/vm.js b/lib/vm.js index 06e72114f2361a..833a6e547b09c2 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -42,7 +42,6 @@ const { const { validateBoolean, validateBuffer, - validateFunction, validateInt32, validateObject, validateOneOf, @@ -55,6 +54,7 @@ const { kVmBreakFirstLineSymbol, } = require('internal/util'); const { + getHostDefinedOptionId, internalCompileFunction, isContext, } = require('internal/vm'); @@ -87,12 +87,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. @@ -104,7 +100,7 @@ class Script extends ContextifyScript { cachedData, produceCachedData, parsingContext, - importModuleDynamically !== undefined); + hostDefinedOptionId); } catch (e) { throw e; /* node-do-not-add-exception-line */ } diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 8b58fa7a2bccb4..65a7acbcfb6bc6 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -787,11 +787,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(); @@ -811,9 +811,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 = @@ -837,12 +836,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); @@ -1201,6 +1194,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()) { @@ -1212,7 +1209,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/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());