Skip to content

Commit

Permalink
lib: add assertion for user ESM execution
Browse files Browse the repository at this point in the history
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: #51748
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
  • Loading branch information
joyeecheung authored and marco-ippolito committed Feb 27, 2024
1 parent 92e7c31 commit 614ca32
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 8 deletions.
7 changes: 2 additions & 5 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ module.exports = {
initializeCJS,
Module,
wrapSafe,
get hasLoadedAnyUserCJSModule() { return hasLoadedAnyUserCJSModule; },
};

const { BuiltinModule } = require('internal/bootstrap/realm');
Expand Down Expand Up @@ -113,6 +112,7 @@ const {
initializeCjsConditions,
loadBuiltinModule,
makeRequireFunction,
setHasStartedUserCJSExecution,
stripBOM,
toRealPath,
} = require('internal/modules/helpers');
Expand All @@ -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: {
Expand Down Expand Up @@ -1363,14 +1360,14 @@ 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);
} else {
result = ReflectApply(compiledWrapper, thisValue,
[exports, require, module, filename, dirname]);
}
hasLoadedAnyUserCJSModule = true;
if (requireDepth === 0) { statCache = null; }
return result;
};
Expand Down
6 changes: 5 additions & 1 deletion lib/internal/modules/esm/module_job.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 };
}
Expand All @@ -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) {
Expand Down
25 changes: 25 additions & 0 deletions lib/internal/modules/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -328,4 +341,16 @@ module.exports = {
normalizeReferrerURL,
stripBOM,
toRealPath,
hasStartedUserCJSExecution() {
return _hasStartedUserCJSExecution;
},
setHasStartedUserCJSExecution() {
_hasStartedUserCJSExecution = true;
},
hasStartedUserESMExecution() {
return _hasStartedUserESMExecution;
},
setHasStartedUserESMExecution() {
_hasStartedUserESMExecution = true;
},
};
8 changes: 6 additions & 2 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 614ca32

Please sign in to comment.