Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 614ca32

Browse filesBrowse files
joyeecheungmarco-ippolito
joyeecheung
authored and
marco-ippolito
committed
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: #51748 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
1 parent 92e7c31 commit 614ca32
Copy full SHA for 614ca32

File tree

Expand file treeCollapse file tree

4 files changed

+38
-8
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+38
-8
lines changed
Open diff view settings
Collapse file

‎lib/internal/modules/cjs/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js
+2-5Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ module.exports = {
7676
initializeCJS,
7777
Module,
7878
wrapSafe,
79-
get hasLoadedAnyUserCJSModule() { return hasLoadedAnyUserCJSModule; },
8079
};
8180

8281
const { BuiltinModule } = require('internal/bootstrap/realm');
@@ -113,6 +112,7 @@ const {
113112
initializeCjsConditions,
114113
loadBuiltinModule,
115114
makeRequireFunction,
115+
setHasStartedUserCJSExecution,
116116
stripBOM,
117117
toRealPath,
118118
} = require('internal/modules/helpers');
@@ -127,9 +127,6 @@ const permission = require('internal/process/permission');
127127
const {
128128
vm_dynamic_import_default_internal,
129129
} = internalBinding('symbols');
130-
// Whether any user-provided CJS modules had been loaded (executed).
131-
// Used for internal assertions.
132-
let hasLoadedAnyUserCJSModule = false;
133130

134131
const {
135132
codes: {
@@ -1363,14 +1360,14 @@ Module.prototype._compile = function(content, filename) {
13631360
const thisValue = exports;
13641361
const module = this;
13651362
if (requireDepth === 0) { statCache = new SafeMap(); }
1363+
setHasStartedUserCJSExecution();
13661364
if (inspectorWrapper) {
13671365
result = inspectorWrapper(compiledWrapper, thisValue, exports,
13681366
require, module, filename, dirname);
13691367
} else {
13701368
result = ReflectApply(compiledWrapper, thisValue,
13711369
[exports, require, module, filename, dirname]);
13721370
}
1373-
hasLoadedAnyUserCJSModule = true;
13741371
if (requireDepth === 0) { statCache = null; }
13751372
return result;
13761373
};
Collapse file

‎lib/internal/modules/esm/module_job.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/module_job.js
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ const {
2727
} = require('internal/source_map/source_map_cache');
2828
const assert = require('internal/assert');
2929
const resolvedPromise = PromiseResolve();
30-
30+
const {
31+
setHasStartedUserESMExecution,
32+
} = require('internal/modules/helpers');
3133
const noop = FunctionPrototype;
3234

3335
let hasPausedEntry = false;
@@ -206,6 +208,7 @@ class ModuleJob {
206208
this.instantiated = PromiseResolve();
207209
const timeout = -1;
208210
const breakOnSigint = false;
211+
setHasStartedUserESMExecution();
209212
this.module.evaluate(timeout, breakOnSigint);
210213
return { __proto__: null, module: this.module };
211214
}
@@ -214,6 +217,7 @@ class ModuleJob {
214217
await this.instantiate();
215218
const timeout = -1;
216219
const breakOnSigint = false;
220+
setHasStartedUserESMExecution();
217221
try {
218222
await this.module.evaluate(timeout, breakOnSigint);
219223
} catch (e) {
Collapse file

‎lib/internal/modules/helpers.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/helpers.js
+25Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,19 @@ function normalizeReferrerURL(referrerName) {
319319
assert.fail('Unreachable code reached by ' + inspect(referrerName));
320320
}
321321

322+
323+
// Whether we have started executing any user-provided CJS code.
324+
// This is set right before we call the wrapped CJS code (not after,
325+
// in case we are half-way in the execution when internals check this).
326+
// Used for internal assertions.
327+
let _hasStartedUserCJSExecution = false;
328+
// Similar to _hasStartedUserCJSExecution but for ESM. This is set
329+
// right before ESM evaluation in the default ESM loader. We do not
330+
// update this during vm SourceTextModule execution because at that point
331+
// some user code must already have been run to execute code via vm
332+
// there is little value checking whether any user JS code is run anyway.
333+
let _hasStartedUserESMExecution = false;
334+
322335
module.exports = {
323336
addBuiltinLibsToObject,
324337
getCjsConditions,
@@ -328,4 +341,16 @@ module.exports = {
328341
normalizeReferrerURL,
329342
stripBOM,
330343
toRealPath,
344+
hasStartedUserCJSExecution() {
345+
return _hasStartedUserCJSExecution;
346+
},
347+
setHasStartedUserCJSExecution() {
348+
_hasStartedUserCJSExecution = true;
349+
},
350+
hasStartedUserESMExecution() {
351+
return _hasStartedUserESMExecution;
352+
},
353+
setHasStartedUserESMExecution() {
354+
_hasStartedUserESMExecution = true;
355+
},
331356
};
Collapse file

‎lib/internal/process/pre_execution.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/pre_execution.js
+6-2Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,12 @@ function setupSymbolDisposePolyfill() {
191191
function setupUserModules(forceDefaultLoader = false) {
192192
initializeCJSLoader();
193193
initializeESMLoader(forceDefaultLoader);
194-
const CJSLoader = require('internal/modules/cjs/loader');
195-
assert(!CJSLoader.hasLoadedAnyUserCJSModule);
194+
const {
195+
hasStartedUserCJSExecution,
196+
hasStartedUserESMExecution,
197+
} = require('internal/modules/helpers');
198+
assert(!hasStartedUserCJSExecution());
199+
assert(!hasStartedUserESMExecution());
196200
// Do not enable preload modules if custom loaders are disabled.
197201
// For example, loader workers are responsible for doing this themselves.
198202
// And preload modules are not supported in ShadowRealm as well.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.