module: fix sync resolve hooks for require with node: prefixes

Previously, when require()-ing builtins with the node: prefix,
the sync resolve hooks were not properly invoked, and load hooks
could not override the builtin's format. This fixes the
handling and enables redirecting prefixed built-ins to on-disk
files and overriding them with other module types via hooks.
PR-URL: #61088
Backport-PR-URL: #62029Fixes: #60005
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Fixes: #61801

Author: joyeecheung

lib/internal/modules/cjs/loader.js

@@ -1086,7 +1086,9 @@ function resolveForCJSWithHooks(specifier, parent, isMain) {
 filename=convertURLToCJSFilename(url);
 }
 
-return{__proto__: null, url, format, filename, parentURL };
+constresult={__proto__: null, url, format, filename, parentURL };
+debug('resolveForCJSWithHooks',specifier,parent?.id,'->',result);
+returnresult;
 }
 
 /**
@@ -1143,24 +1145,29 @@ function getDefaultLoad(url, filename) {
  * @param {string} id The module ID (without the node: prefix)
  * @param {string} url The module URL (with the node: prefix)
  * @param {string} format Format from resolution.
- * @returns {any} If there are no load hooks or the load hooks do not override the format of the
- * builtin, load and return the exports of the builtin. Otherwise, return undefined.
+ * @returns {{builtinExports: any, resultFromHook: undefined|ModuleLoadResult}} If there are no load
+ * hooks or the load hooks do not override the format of the builtin, load and return the exports
+ * of the builtin module. Otherwise, return the loadResult for the caller to continue loading.
  */
 functionloadBuiltinWithHooks(id,url,format){
+letresultFromHook;
 if(loadHooks.length){
 url??=`node:${id}`;
+debug('loadBuiltinWithHooks ',loadHooks.length,id,url,format);
 // TODO(joyeecheung): do we really want to invoke the load hook for the builtins?
-constloadResult=loadWithHooks(url,format||'builtin',/* importAttributes */undefined,
-getCjsConditionsArray(),getDefaultLoad(url,id),validateLoadStrict);
-if(loadResult.format&&loadResult.format!=='builtin'){
-returnundefined;// Format has been overridden, return undefined for the caller to continue loading.
+resultFromHook=loadWithHooks(url,format||'builtin',/* importAttributes */undefined,
+getCjsConditionsArray(),getDefaultLoad(url,id),validateLoadStrict);
+if(resultFromHook.format&&resultFromHook.format!=='builtin'){
+debug('loadBuiltinWithHooks overriding module',id,url,resultFromHook);
+// Format has been overridden, return result for the caller to continue loading.
+return{builtinExports: undefined, resultFromHook };
 }
 }
 
 // No hooks or the hooks have not overridden the format. Load it as a builtin module and return the
 // exports.
 constmod=loadBuiltinModule(id);
-returnmod.exports;
+return{builtinExports: mod.exports,resultFromHook: undefined};
 }
 
 /**
@@ -1197,47 +1204,64 @@ Module._load = function(request, parent, isMain) {
 }
 }
 
-const{ url, format, filename }=resolveForCJSWithHooks(request,parent,isMain);
+constresolveResult=resolveForCJSWithHooks(request,parent,isMain);
+let{ format }=resolveResult;
+const{ url, filename }=resolveResult;
 
+letresultFromLoadHook;
 // For backwards compatibility, if the request itself starts with node:, load it before checking
 // Module._cache. Otherwise, load it after the check.
-if(StringPrototypeStartsWith(request,'node:')){
-constresult=loadBuiltinWithHooks(filename,url,format);
-if(result){
-returnresult;
+// TODO(joyeecheung): a more sensible handling is probably, if there are hooks, always go through the hooks
+// first before checking the cache. Otherwise, check the cache first, then proceed to default loading.
+if(request===url&&StringPrototypeStartsWith(request,'node:')){
+constnormalized=BuiltinModule.normalizeRequirableId(request);
+if(normalized){// It's a builtin module.
+const{ resultFromHook, builtinExports }=loadBuiltinWithHooks(normalized,url,format);
+if(builtinExports){
+returnbuiltinExports;
+}
+// The format of the builtin has been overridden by user hooks. Continue loading.
+resultFromLoadHook=resultFromHook;
+format=resultFromLoadHook.format;
 }
-// The format of the builtin has been overridden by user hooks. Continue loading.
 }
 
-constcachedModule=Module._cache[filename];
-if(cachedModule!==undefined){
-updateChildren(parent,cachedModule,true);
-if(cachedModule.loaded){
-returncachedModule.exports;
-}
-// If it's not cached by the ESM loader, the loading request
-// comes from required CJS, and we can consider it a circular
-// dependency when it's cached.
-if(!cachedModule[kIsCachedByESMLoader]){
-returngetExportsForCircularRequire(cachedModule);
-}
-// If it's cached by the ESM loader as a way to indirectly pass
-// the module in to avoid creating it twice, the loading request
-// came from imported CJS. In that case use the kModuleCircularVisited
-// to determine if it's loading or not.
-if(cachedModule[kModuleCircularVisited]){
-returngetExportsForCircularRequire(cachedModule);
+// If load hooks overrides the format for a built-in, bypass the cache.
+letcachedModule;
+if(resultFromLoadHook===undefined){
+cachedModule=Module._cache[filename];
+debug('Module._load checking cache for',filename,!!cachedModule);
+if(cachedModule!==undefined){
+updateChildren(parent,cachedModule,true);
+if(cachedModule.loaded){
+returncachedModule.exports;
+}
+// If it's not cached by the ESM loader, the loading request
+// comes from required CJS, and we can consider it a circular
+// dependency when it's cached.
+if(!cachedModule[kIsCachedByESMLoader]){
+returngetExportsForCircularRequire(cachedModule);
+}
+// If it's cached by the ESM loader as a way to indirectly pass
+// the module in to avoid creating it twice, the loading request
+// came from imported CJS. In that case use the kModuleCircularVisited
+// to determine if it's loading or not.
+if(cachedModule[kModuleCircularVisited]){
+returngetExportsForCircularRequire(cachedModule);
+}
+// This is an ESM loader created cache entry, mark it as visited and fallthrough to loading the module.
+cachedModule[kModuleCircularVisited]=true;
 }
-// This is an ESM loader created cache entry, mark it as visited and fallthrough to loading the module.
-cachedModule[kModuleCircularVisited]=true;
 }
 
-if(BuiltinModule.canBeRequiredWithoutScheme(filename)){
-constresult=loadBuiltinWithHooks(filename,url,format);
-if(result){
-returnresult;
+if(resultFromLoadHook===undefined&&BuiltinModule.canBeRequiredWithoutScheme(filename)){
+const{ resultFromHook, builtinExports }=loadBuiltinWithHooks(filename,url,format);
+if(builtinExports){
+returnbuiltinExports;
 }
 // The format of the builtin has been overridden by user hooks. Continue loading.
+resultFromLoadHook=resultFromHook;
+format=resultFromLoadHook.format;
 }
 
 // Don't call updateChildren(), Module constructor already does.
@@ -1252,6 +1276,9 @@ Module._load = function(request, parent, isMain) {
 }else{
 module[kIsMainSymbol]=false;
 }
+if(resultFromLoadHook!==undefined){
+module[kModuleSource]=resultFromLoadHook.source;
+}
 
 reportModuleToWatchMode(filename);
 Module._cache[filename]=module;
@@ -1436,6 +1463,17 @@ function createEsmNotFoundErr(request, path) {
 returnerr;
 }
 
+functiongetExtensionForFormat(format){
+switch(format){
+case'addon':
+return'.node';
+case'json':
+return'.json';
+default:
+return'.js';
+}
+}
+
 /**
  * Given a file name, pass it to the proper extension handler.
  * @param {string} filename The `require` specifier
@@ -1447,7 +1485,13 @@ Module.prototype.load = function(filename) {
 this.filename??=filename;
 this.paths??=Module._nodeModulePaths(path.dirname(filename));
 
-constextension=findLongestRegisteredExtension(filename);
+// If the format is already overridden by hooks, convert that back to extension.
+letextension;
+if(this[kFormat]!==undefined){
+extension=getExtensionForFormat(this[kFormat]);
+}else{
+extension=findLongestRegisteredExtension(filename);
+}
 
 Module._extensions[extension](this,filename);
 this.loaded=true;

test/fixtures/module-hooks/redirected-zlib.mjs

@@ -0,0 +1 @@
+exportconsturl=import.meta.url;

test/module-hooks/test-module-hooks-load-builtin-override-commonjs.js

@@ -0,0 +1,37 @@
+'use strict';
+
+// This tests that load hooks can override the format of builtin modules
+// to 'commonjs' format.
+constcommon=require('../common');
+constassert=require('assert');
+const{ registerHooks }=require('module');
+
+// Pick a builtin that's unlikely to be loaded already - like zlib.
+assert(!process.moduleLoadList.includes('NativeModule zlib'));
+
+consthook=registerHooks({
+load: common.mustCall(functionload(url,context,nextLoad){
+// Only intercept zlib builtin
+if(url==='node:zlib'){
+// Return a different format to override the builtin
+return{
+source: 'exports.custom_zlib = "overridden by load hook";',
+format: 'commonjs',
+shortCircuit: true,
+};
+}
+returnnextLoad(url,context);
+},2),// Called twice: once for 'zlib', once for 'node:zlib'
+});
+
+// Test: Load hook overrides builtin format to commonjs
+constzlib=require('zlib');
+assert.strictEqual(zlib.custom_zlib,'overridden by load hook');
+assert.strictEqual(typeofzlib.createGzip,'undefined');// Original zlib API should not be available
+
+// Test with node: prefix
+constzlib2=require('node:zlib');
+assert.strictEqual(zlib2.custom_zlib,'overridden by load hook');
+assert.strictEqual(typeofzlib2.createGzip,'undefined');
+
+hook.deregister();

test/module-hooks/test-module-hooks-load-builtin-override-json.js

@@ -0,0 +1,37 @@
+'use strict';
+
+// This tests that load hooks can override the format of builtin modules
+// to 'json' format.
+constcommon=require('../common');
+constassert=require('assert');
+const{ registerHooks }=require('module');
+
+// Pick a builtin that's unlikely to be loaded already - like zlib.
+assert(!process.moduleLoadList.includes('NativeModule zlib'));
+
+consthook=registerHooks({
+load: common.mustCall(functionload(url,context,nextLoad){
+// Only intercept zlib builtin
+if(url==='node:zlib'){
+// Return JSON format to override the builtin
+return{
+source: JSON.stringify({custom_zlib: 'JSON overridden zlib'}),
+format: 'json',
+shortCircuit: true,
+};
+}
+returnnextLoad(url,context);
+},2),// Called twice: once for 'zlib', once for 'node:zlib'
+});
+
+// Test: Load hook overrides builtin format to json
+constzlib=require('zlib');
+assert.strictEqual(zlib.custom_zlib,'JSON overridden zlib');
+assert.strictEqual(typeofzlib.createGzip,'undefined');// Original zlib API should not be available
+
+// Test with node: prefix
+constzlib2=require('node:zlib');
+assert.strictEqual(zlib2.custom_zlib,'JSON overridden zlib');
+assert.strictEqual(typeofzlib2.createGzip,'undefined');
+
+hook.deregister();

test/module-hooks/test-module-hooks-load-builtin-override-module.js

@@ -0,0 +1,41 @@
+'use strict';
+
+// This tests that load hooks can override the format of builtin modules
+// to 'module', and require() can load them.
+constcommon=require('../common');
+constassert=require('assert');
+const{ registerHooks }=require('module');
+
+// Pick a builtin that's unlikely to be loaded already - like zlib.
+assert(!process.moduleLoadList.includes('NativeModule zlib'));
+
+consthook=registerHooks({
+load: common.mustCall(functionload(url,context,nextLoad){
+// Only intercept zlib builtin
+if(url==='node:zlib'){
+// Return ES module format to override the builtin
+// Note: For require() to work with ESM, we need to export 'module.exports'
+return{
+source: `const exports = { custom_zlib: "ESM overridden zlib" };
+ export default exports;
+ export { exports as 'module.exports' };`,
+format: 'module',
+shortCircuit: true,
+};
+}
+returnnextLoad(url,context);
+},2),// Called twice: once for 'zlib', once for 'node:zlib'
+});
+
+// Test: Load hook overrides builtin format to module.
+// With the 'module.exports' export, require() should work
+constzlib=require('zlib');
+assert.strictEqual(zlib.custom_zlib,'ESM overridden zlib');
+assert.strictEqual(typeofzlib.createGzip,'undefined');// Original zlib API should not be available
+
+// Test with node: prefix
+constzlib2=require('node:zlib');
+assert.strictEqual(zlib2.custom_zlib,'ESM overridden zlib');
+assert.strictEqual(typeofzlib2.createGzip,'undefined');
+
+hook.deregister();

test/module-hooks/test-module-hooks-resolve-builtin-on-disk-require-with-prefix.js

@@ -0,0 +1,33 @@
+'use strict';
+
+// This tests that builtins can be redirected to a local file when they are prefixed
+// with `node:`.
+require('../common');
+
+constassert=require('assert');
+const{ registerHooks }=require('module');
+constfixtures=require('../common/fixtures');
+
+// This tests that builtins can be redirected to a local file.
+// Pick a builtin that's unlikely to be loaded already - like zlib.
+assert(!process.moduleLoadList.includes('NativeModule zlib'));
+
+consthook=registerHooks({
+resolve(specifier,context,nextLoad){
+specifier=specifier.replaceAll('node:','');
+return{
+url: fixtures.fileURL('module-hooks',`redirected-${specifier}.js`).href,
+shortCircuit: true,
+};
+},
+});
+
+// Check assert, which is already loaded.
+// eslint-disable-next-line node-core/must-call-assert
+assert.strictEqual(require('node:assert').exports_for_test,'redirected assert');
+// Check zlib, which is not yet loaded.
+assert.strictEqual(require('node:zlib').exports_for_test,'redirected zlib');
+// Check fs, which is redirected to an ESM
+assert.strictEqual(require('node:fs').exports_for_test,'redirected fs');
+
+hook.deregister();

test/module-hooks/test-module-hooks-resolve-load-builtin-override-both-prefix.js

@@ -0,0 +1,36 @@
+'use strict';
+
+// This tests the interaction between resolve and load hooks for builtins with the
+// `node:` prefix.
+constcommon=require('../common');
+constassert=require('assert');
+const{ registerHooks }=require('module');
+constfixtures=require('../common/fixtures');
+
+// Pick a builtin that's unlikely to be loaded already - like zlib.
+assert(!process.moduleLoadList.includes('NativeModule zlib'));
+
+constredirectedURL=fixtures.fileURL('module-hooks/redirected-zlib.js').href;
+
+registerHooks({
+resolve: common.mustCall(functionresolve(specifier,context,nextResolve){
+assert.strictEqual(specifier,'node:zlib');
+return{
+url: redirectedURL,
+format: 'module',
+shortCircuit: true,
+};
+}),
+
+load: common.mustCall(functionload(url,context,nextLoad){
+assert.strictEqual(url,redirectedURL);
+return{
+source: 'export const loadURL = import.meta.url;',
+format: 'module',
+shortCircuit: true,
+};
+}),
+});
+
+constzlib=require('node:zlib');
+assert.strictEqual(zlib.loadURL,redirectedURL);