From 614ca327c8facc169cbe8caa62e3fa6396cc052e 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 802e2f6752a693..40eefff40c5b65 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: { @@ -1363,6 +1360,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); @@ -1370,7 +1368,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 33576c5893600b..05db7f3867efe2 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 b94b066ed19c57..15ec0870d1b1b5 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -191,8 +191,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.