Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commit51b88fa

Browse files
joyeecheungmarco-ippolito
authored andcommitted
module: disallow CJS <-> ESM edges in a cycle from require(esm)
This patch disallows CJS <-> ESM edges when they come fromrequire(esm) requested in ESM evalaution.Drive-by: don't reuse the cache for imported CJS modules to stashsource code of required ESM because the former is also used forcycle detection.PR-URL:#52264Backport-PR-URL:#53500Fixes:#52145Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>Reviewed-By: Guy Bedford <guybedford@gmail.com>Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent4dae68c commit51b88fa

File tree

28 files changed

+499
-47
lines changed

28 files changed

+499
-47
lines changed

‎doc/api/errors.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2521,6 +2521,21 @@ Accessing `Object.prototype.__proto__` has been forbidden using
25212521
[`Object.setPrototypeOf`][] should be used to get and set the prototype of an
25222522
object.
25232523

2524+
<aid="ERR_REQUIRE_CYCLE_MODULE"></a>
2525+
2526+
###`ERR_REQUIRE_CYCLE_MODULE`
2527+
2528+
>Stability: 1 - Experimental
2529+
2530+
When trying to`require()` a[ES Module][] under`--experimental-require-module`,
2531+
a CommonJS to ESM or ESM to CommonJS edge participates in an immediate cycle.
2532+
This is not allowed because ES Modules cannot be evaluated while they are
2533+
already being evaluated.
2534+
2535+
To avoid the cycle, the`require()` call involved in a cycle should not happen
2536+
at the top-level of either a ES Module (via`createRequire()`) or a CommonJS
2537+
module, and should be done lazily in an inner function.
2538+
25242539
<aid="ERR_REQUIRE_ASYNC_MODULE"></a>
25252540

25262541
###`ERR_REQUIRE_ASYNC_MODULE`

‎lib/internal/errors.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1692,6 +1692,7 @@ E('ERR_PARSE_ARGS_UNKNOWN_OPTION', (option, allowPositionals) => {
16921692
E('ERR_PERFORMANCE_INVALID_TIMESTAMP',
16931693
'%d is not a valid timestamp',TypeError);
16941694
E('ERR_PERFORMANCE_MEASURE_INVALID_OPTIONS','%s',TypeError);
1695+
E('ERR_REQUIRE_CYCLE_MODULE','%s',Error);
16951696
E('ERR_REQUIRE_ESM',
16961697
function(filename,hasEsmSyntax,parentPath=null,packageJsonPath=null){
16971698
hideInternalStackFrames(this);

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

Lines changed: 56 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -63,24 +63,33 @@ const {
6363
Symbol,
6464
}=primordials;
6565

66+
const{ kEvaluated}=internalBinding('module_wrap');
67+
6668
// Map used to store CJS parsing data or for ESM loading.
67-
constcjsSourceCache=newSafeWeakMap();
69+
constimportedCJSCache=newSafeWeakMap();
6870
/**
6971
* Map of already-loaded CJS modules to use.
7072
*/
7173
constcjsExportsCache=newSafeWeakMap();
74+
constrequiredESMSourceCache=newSafeWeakMap();
7275

76+
constkIsMainSymbol=Symbol('kIsMainSymbol');
77+
constkIsCachedByESMLoader=Symbol('kIsCachedByESMLoader');
78+
constkRequiredModuleSymbol=Symbol('kRequiredModuleSymbol');
79+
constkIsExecuting=Symbol('kIsExecuting');
7380
// Set first due to cycle with ESM loader functions.
7481
module.exports={
7582
cjsExportsCache,
76-
cjsSourceCache,
83+
importedCJSCache,
7784
initializeCJS,
7885
Module,
7986
wrapSafe,
87+
kIsMainSymbol,
88+
kIsCachedByESMLoader,
89+
kRequiredModuleSymbol,
90+
kIsExecuting,
8091
};
8192

82-
constkIsMainSymbol=Symbol('kIsMainSymbol');
83-
8493
const{ BuiltinModule}=require('internal/bootstrap/realm');
8594
const{
8695
maybeCacheSourceMap,
@@ -137,6 +146,7 @@ const {
137146
codes:{
138147
ERR_INVALID_ARG_VALUE,
139148
ERR_INVALID_MODULE_SPECIFIER,
149+
ERR_REQUIRE_CYCLE_MODULE,
140150
ERR_REQUIRE_ESM,
141151
ERR_UNKNOWN_BUILTIN_MODULE,
142152
},
@@ -942,6 +952,16 @@ const CircularRequirePrototypeWarningProxy = new Proxy({}, {
942952
*@param {Module} module The module instance
943953
*/
944954
functiongetExportsForCircularRequire(module){
955+
constrequiredESM=module[kRequiredModuleSymbol];
956+
if(requiredESM&&requiredESM.getStatus()!==kEvaluated){
957+
letmessage=`Cannot require() ES Module${module.id} in a cycle.`;
958+
constparent=moduleParentCache.get(module);
959+
if(parent){
960+
message+=` (from${parent.filename})`;
961+
}
962+
thrownewERR_REQUIRE_CYCLE_MODULE(message);
963+
}
964+
945965
if(module.exports&&
946966
!isProxy(module.exports)&&
947967
ObjectGetPrototypeOf(module.exports)===ObjectPrototype&&
@@ -1009,11 +1029,21 @@ Module._load = function(request, parent, isMain) {
10091029
if(cachedModule!==undefined){
10101030
updateChildren(parent,cachedModule,true);
10111031
if(!cachedModule.loaded){
1012-
constparseCachedModule=cjsSourceCache.get(cachedModule);
1013-
if(!parseCachedModule||parseCachedModule.loaded){
1032+
// If it's not cached by the ESM loader, the loading request
1033+
// comes from required CJS, and we can consider it a circular
1034+
// dependency when it's cached.
1035+
if(!cachedModule[kIsCachedByESMLoader]){
10141036
returngetExportsForCircularRequire(cachedModule);
10151037
}
1016-
parseCachedModule.loaded=true;
1038+
// If it's cached by the ESM loader as a way to indirectly pass
1039+
// the module in to avoid creating it twice, the loading request
1040+
// come from imported CJS. In that case use the importedCJSCache
1041+
// to determine if it's loading or not.
1042+
constimportedCJSMetadata=importedCJSCache.get(cachedModule);
1043+
if(importedCJSMetadata.loading){
1044+
returngetExportsForCircularRequire(cachedModule);
1045+
}
1046+
importedCJSMetadata.loading=true;
10171047
}else{
10181048
returncachedModule.exports;
10191049
}
@@ -1027,18 +1057,21 @@ Module._load = function(request, parent, isMain) {
10271057
// Don't call updateChildren(), Module constructor already does.
10281058
constmodule=cachedModule||newModule(filename,parent);
10291059

1030-
if(isMain){
1031-
setOwnProperty(process,'mainModule',module);
1032-
setOwnProperty(module.require,'main',process.mainModule);
1033-
module.id='.';
1034-
module[kIsMainSymbol]=true;
1035-
}else{
1036-
module[kIsMainSymbol]=false;
1037-
}
1060+
if(!cachedModule){
1061+
if(isMain){
1062+
setOwnProperty(process,'mainModule',module);
1063+
setOwnProperty(module.require,'main',process.mainModule);
1064+
module.id='.';
1065+
module[kIsMainSymbol]=true;
1066+
}else{
1067+
module[kIsMainSymbol]=false;
1068+
}
10381069

1039-
reportModuleToWatchMode(filename);
1070+
reportModuleToWatchMode(filename);
1071+
Module._cache[filename]=module;
1072+
module[kIsCachedByESMLoader]=false;
1073+
}
10401074

1041-
Module._cache[filename]=module;
10421075
if(parent!==undefined){
10431076
relativeResolveCache[relResolveCacheIdentifier]=filename;
10441077
}
@@ -1280,7 +1313,7 @@ function loadESMFromCJS(mod, filename) {
12801313
constisMain=mod[kIsMainSymbol];
12811314
// TODO(joyeecheung): we may want to invent optional special handling for default exports here.
12821315
// For now, it's good enough to be identical to what `import()` returns.
1283-
mod.exports=cascadedLoader.importSyncForRequire(filename,source,isMain);
1316+
mod.exports=cascadedLoader.importSyncForRequire(mod,filename,source,isMain,moduleParentCache.get(mod));
12841317
}
12851318

12861319
/**
@@ -1366,7 +1399,7 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
13661399
// Only modules being require()'d really need to avoid TLA.
13671400
if(loadAsESM){
13681401
// Pass the source into the .mjs extension handler indirectly through the cache.
1369-
cjsSourceCache.set(this,{source:content});
1402+
requiredESMSourceCache.set(this,content);
13701403
loadESMFromCJS(this,filename);
13711404
return;
13721405
}
@@ -1407,13 +1440,15 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
14071440
constmodule=this;
14081441
if(requireDepth===0){statCache=newSafeMap();}
14091442
setHasStartedUserCJSExecution();
1443+
this[kIsExecuting]=true;
14101444
if(inspectorWrapper){
14111445
result=inspectorWrapper(compiledWrapper,thisValue,exports,
14121446
require,module,filename,dirname);
14131447
}else{
14141448
result=ReflectApply(compiledWrapper,thisValue,
14151449
[exports,require,module,filename,dirname]);
14161450
}
1451+
this[kIsExecuting]=false;
14171452
if(requireDepth===0){statCache=null;}
14181453
returnresult;
14191454
};
@@ -1425,15 +1460,15 @@ Module.prototype._compile = function(content, filename, loadAsESM = false) {
14251460
*@returns {string}
14261461
*/
14271462
functiongetMaybeCachedSource(mod,filename){
1428-
constcached=cjsSourceCache.get(mod);
1463+
constcached=importedCJSCache.get(mod);
14291464
letcontent;
14301465
if(cached?.source){
14311466
content=cached.source;
14321467
cached.source=undefined;
14331468
}else{
14341469
// TODO(joyeecheung): we can read a buffer instead to speed up
14351470
// compilation.
1436-
content=fs.readFileSync(filename,'utf8');
1471+
content=requiredESMSourceCache.get(mod)??fs.readFileSync(filename,'utf8');
14371472
}
14381473
returncontent;
14391474
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,11 @@ async function defaultLoad(url, context = kEmptyObject) {
152152

153153
validateAttributes(url,format,importAttributes);
154154

155+
// Use the synchronous commonjs translator which can deal with cycles.
156+
if(format==='commonjs'&&getOptionValue('--experimental-require-module')){
157+
format='commonjs-sync';
158+
}
159+
155160
return{
156161
__proto__:null,
157162
format,
@@ -201,6 +206,11 @@ function defaultLoadSync(url, context = kEmptyObject) {
201206

202207
validateAttributes(url,format,importAttributes);
203208

209+
// Use the synchronous commonjs translator which can deal with cycles.
210+
if(format==='commonjs'&&getOptionValue('--experimental-require-module')){
211+
format='commonjs-sync';
212+
}
213+
204214
return{
205215
__proto__:null,
206216
format,

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

Lines changed: 65 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
'use strict';
22

33
// This is needed to avoid cycles in esm/resolve <-> cjs/loader
4-
require('internal/modules/cjs/loader');
4+
const{
5+
kIsExecuting,
6+
kRequiredModuleSymbol,
7+
}=require('internal/modules/cjs/loader');
58

69
const{
710
ArrayPrototypeJoin,
@@ -15,8 +18,11 @@ const {
1518
hardenRegExp,
1619
}=primordials;
1720

21+
const{ imported_cjs_symbol}=internalBinding('symbols');
22+
1823
constassert=require('internal/assert');
1924
const{
25+
ERR_REQUIRE_CYCLE_MODULE,
2026
ERR_REQUIRE_ESM,
2127
ERR_NETWORK_IMPORT_DISALLOWED,
2228
ERR_UNKNOWN_MODULE_FORMAT,
@@ -30,7 +36,10 @@ const {
3036
}=require('internal/modules/esm/utils');
3137
const{ kImplicitAssertType}=require('internal/modules/esm/assert');
3238
const{ canParse}=internalBinding('url');
33-
const{ ModuleWrap}=internalBinding('module_wrap');
39+
const{ ModuleWrap, kEvaluating, kEvaluated}=internalBinding('module_wrap');
40+
const{
41+
urlToFilename,
42+
}=require('internal/modules/helpers');
3443
letdefaultResolve,defaultLoad,defaultLoadSync,importMetaInitializer;
3544

3645
/**
@@ -248,17 +257,36 @@ class ModuleLoader {
248257
/**
249258
* This constructs (creates, instantiates and evaluates) a module graph that
250259
* is require()'d.
260+
*@param {import('../cjs/loader.js').Module} mod CJS module wrapper of the ESM.
251261
*@param {string} filename Resolved filename of the module being require()'d
252262
*@param {string} source Source code. TODO(joyeecheung): pass the raw buffer.
253263
*@param {string} isMain Whether this module is a main module.
254-
*@returns {ModuleNamespaceObject}
264+
*@param {import('../cjs/loader.js').Module|undefined} parent Parent module, if any.
265+
*@returns {{ModuleWrap}}
255266
*/
256-
importSyncForRequire(filename,source,isMain){
267+
importSyncForRequire(mod,filename,source,isMain,parent){
257268
consturl=pathToFileURL(filename).href;
258269
letjob=this.loadCache.get(url,kImplicitAssertType);
259-
// This module is already loaded, check whether it's synchronous and return the
260-
// namespace.
270+
// This module job is already created:
271+
// 1. If it was loaded by `require()` before, at this point the instantiation
272+
// is already completed and we can check the whether it is in a cycle
273+
// (in that case the module status is kEvaluaing), and whether the
274+
// required graph is synchronous.
275+
// 2. If it was loaded by `import` before, only allow it if it's already evaluated
276+
// to forbid cycles.
277+
// TODO(joyeecheung): ensure that imported synchronous graphs are evaluated
278+
// synchronously so that any previously imported synchronous graph is already
279+
// evaluated at this point.
261280
if(job!==undefined){
281+
mod[kRequiredModuleSymbol]=job.module;
282+
if(job.module.getStatus()!==kEvaluated){
283+
constparentFilename=urlToFilename(parent?.filename);
284+
letmessage=`Cannot require() ES Module${filename} in a cycle.`;
285+
if(parentFilename){
286+
message+=` (from${parentFilename})`;
287+
}
288+
thrownewERR_REQUIRE_CYCLE_MODULE(message);
289+
}
262290
returnjob.module.getNamespaceSync();
263291
}
264292
// TODO(joyeecheung): refactor this so that we pre-parse in C++ and hit the
@@ -270,6 +298,7 @@ class ModuleLoader {
270298
const{ ModuleJobSync}=require('internal/modules/esm/module_job');
271299
job=newModuleJobSync(this,url,kEmptyObject,wrap,isMain,inspectBrk);
272300
this.loadCache.set(url,kImplicitAssertType,job);
301+
mod[kRequiredModuleSymbol]=job.module;
273302
returnjob.runSync().namespace;
274303
}
275304

@@ -304,19 +333,29 @@ class ModuleLoader {
304333
constresolvedImportAttributes=resolveResult.importAttributes??importAttributes;
305334
letjob=this.loadCache.get(url,resolvedImportAttributes.type);
306335
if(job!==undefined){
307-
// This module is previously imported before. We will return the module now and check
308-
// asynchronicity of the entire graph later, after the graph is instantiated.
336+
// This module is being evaluated, which means it's imported in a previous link
337+
// in a cycle.
338+
if(job.module.getStatus()===kEvaluating){
339+
constparentFilename=urlToFilename(parentURL);
340+
letmessage=`Cannot import Module${specifier} in a cycle.`;
341+
if(parentFilename){
342+
message+=` (from${parentFilename})`;
343+
}
344+
thrownewERR_REQUIRE_CYCLE_MODULE(message);
345+
}
346+
// Othersie the module could be imported before but the evaluation may be already
347+
// completed (e.g. the require call is lazy) so it's okay. We will return the
348+
// module now and check asynchronicity of the entire graph later, after the
349+
// graph is instantiated.
309350
returnjob.module;
310351
}
311352

312353
defaultLoadSync??=require('internal/modules/esm/load').defaultLoadSync;
313354
constloadResult=defaultLoadSync(url,{ format, importAttributes});
314355
const{ responseURL, source}=loadResult;
315-
let{format:finalFormat}=loadResult;
356+
const{format:finalFormat}=loadResult;
316357
this.validateLoadResult(url,finalFormat);
317-
if(finalFormat==='commonjs'){
318-
finalFormat='commonjs-sync';
319-
}elseif(finalFormat==='wasm'){
358+
if(finalFormat==='wasm'){
320359
assert.fail('WASM is currently unsupported by require(esm)');
321360
}
322361

@@ -333,6 +372,20 @@ class ModuleLoader {
333372
process.send({'watch:import':[url]});
334373
}
335374

375+
constcjsModule=wrap[imported_cjs_symbol];
376+
if(cjsModule){
377+
assert(finalFormat==='commonjs-sync');
378+
// Check if the ESM initiating import CJS is being required by the same CJS module.
379+
if(cjsModule&&cjsModule[kIsExecuting]){
380+
constparentFilename=urlToFilename(parentURL);
381+
letmessage=`Cannot import CommonJS Module${specifier} in a cycle.`;
382+
if(parentFilename){
383+
message+=` (from${parentFilename})`;
384+
}
385+
thrownewERR_REQUIRE_CYCLE_MODULE(message);
386+
}
387+
}
388+
336389
constinspectBrk=(isMain&&getOptionValue('--inspect-brk'));
337390
const{ ModuleJobSync}=require('internal/modules/esm/module_job');
338391
job=newModuleJobSync(this,url,importAttributes,wrap,isMain,inspectBrk);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp