From d85d68ee0e842bc9a5e76ce1b6aa1480e709dabd Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 21 Feb 2024 02:59:08 +0100 Subject: [PATCH] lib: add assertion for user ESM execution Previously we only had an internal assertion to ensure certain code is executed before any user-provided CJS is run. This patch adds another assertion for ESM. Note that this internal state is not updated during source text module execution via vm because to run any code via vm, some user JS code must have already been executed anyway. In addition this patch moves the states into internal/modules/helpers to avoid circular dependencies. Also moves toggling the states to true *right before* user code execution instead of after in case we are half-way in the execution when internals try to check them. PR-URL: https://github.com/nodejs/node/pull/51748 Reviewed-By: Antoine du Hamel Reviewed-By: Geoffrey Booth --- lib/internal/modules/cjs/loader.js | 7 ++----- lib/internal/modules/esm/module_job.js | 6 +++++- lib/internal/modules/helpers.js | 25 +++++++++++++++++++++++++ lib/internal/process/pre_execution.js | 8 ++++++-- 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index d60fa002cd5411..2c48ff9512c225 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -76,7 +76,6 @@ module.exports = { initializeCJS, Module, wrapSafe, - get hasLoadedAnyUserCJSModule() { return hasLoadedAnyUserCJSModule; }, }; const { BuiltinModule } = require('internal/bootstrap/realm'); @@ -113,6 +112,7 @@ const { initializeCjsConditions, loadBuiltinModule, makeRequireFunction, + setHasStartedUserCJSExecution, stripBOM, toRealPath, } = require('internal/modules/helpers'); @@ -127,9 +127,6 @@ const permission = require('internal/process/permission'); const { vm_dynamic_import_default_internal, } = internalBinding('symbols'); -// Whether any user-provided CJS modules had been loaded (executed). -// Used for internal assertions. -let hasLoadedAnyUserCJSModule = false; const { codes: { @@ -1364,6 +1361,7 @@ Module.prototype._compile = function(content, filename) { const thisValue = exports; const module = this; if (requireDepth === 0) { statCache = new SafeMap(); } + setHasStartedUserCJSExecution(); if (inspectorWrapper) { result = inspectorWrapper(compiledWrapper, thisValue, exports, require, module, filename, dirname); @@ -1371,7 +1369,6 @@ Module.prototype._compile = function(content, filename) { result = ReflectApply(compiledWrapper, thisValue, [exports, require, module, filename, dirname]); } - hasLoadedAnyUserCJSModule = true; if (requireDepth === 0) { statCache = null; } return result; }; diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 83c23456e05f10..ba62fd4361a9a6 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -27,7 +27,9 @@ const { } = require('internal/source_map/source_map_cache'); const assert = require('internal/assert'); const resolvedPromise = PromiseResolve(); - +const { + setHasStartedUserESMExecution, +} = require('internal/modules/helpers'); const noop = FunctionPrototype; let hasPausedEntry = false; @@ -206,6 +208,7 @@ class ModuleJob { this.instantiated = PromiseResolve(); const timeout = -1; const breakOnSigint = false; + setHasStartedUserESMExecution(); this.module.evaluate(timeout, breakOnSigint); return { __proto__: null, module: this.module }; } @@ -214,6 +217,7 @@ class ModuleJob { await this.instantiate(); const timeout = -1; const breakOnSigint = false; + setHasStartedUserESMExecution(); try { await this.module.evaluate(timeout, breakOnSigint); } catch (e) { diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js index 0eabb94848598e..665019e93d1ade 100644 --- a/lib/internal/modules/helpers.js +++ b/lib/internal/modules/helpers.js @@ -319,6 +319,19 @@ function normalizeReferrerURL(referrerName) { assert.fail('Unreachable code reached by ' + inspect(referrerName)); } + +// Whether we have started executing any user-provided CJS code. +// This is set right before we call the wrapped CJS code (not after, +// in case we are half-way in the execution when internals check this). +// Used for internal assertions. +let _hasStartedUserCJSExecution = false; +// Similar to _hasStartedUserCJSExecution but for ESM. This is set +// right before ESM evaluation in the default ESM loader. We do not +// update this during vm SourceTextModule execution because at that point +// some user code must already have been run to execute code via vm +// there is little value checking whether any user JS code is run anyway. +let _hasStartedUserESMExecution = false; + module.exports = { addBuiltinLibsToObject, getCjsConditions, @@ -328,4 +341,16 @@ module.exports = { normalizeReferrerURL, stripBOM, toRealPath, + hasStartedUserCJSExecution() { + return _hasStartedUserCJSExecution; + }, + setHasStartedUserCJSExecution() { + _hasStartedUserCJSExecution = true; + }, + hasStartedUserESMExecution() { + return _hasStartedUserESMExecution; + }, + setHasStartedUserESMExecution() { + _hasStartedUserESMExecution = true; + }, }; diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index 6331c338f4c649..9d80d2a55c72d2 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -190,8 +190,12 @@ function setupSymbolDisposePolyfill() { function setupUserModules(forceDefaultLoader = false) { initializeCJSLoader(); initializeESMLoader(forceDefaultLoader); - const CJSLoader = require('internal/modules/cjs/loader'); - assert(!CJSLoader.hasLoadedAnyUserCJSModule); + const { + hasStartedUserCJSExecution, + hasStartedUserESMExecution, + } = require('internal/modules/helpers'); + assert(!hasStartedUserCJSExecution()); + assert(!hasStartedUserESMExecution()); // Do not enable preload modules if custom loaders are disabled. // For example, loader workers are responsible for doing this themselves. // And preload modules are not supported in ShadowRealm as well.