esm: Ensure custom loader resolved "url" is properly validated

doc/api/errors.md

@@ -1204,10 +1204,23 @@ An invalid `options.protocol` was passed.
 Both `breakEvalOnSigint` and `eval` options were set in the REPL config, which
 is not supported.
 
+<a id="ERR_INVALID_RETURN_PROPERTY"></a>
+### ERR_INVALID_RETURN_PROPERTY
+
+Thrown in case a function option does not provide a valid value for one of its
+returned object properties on execution.
+
+<a id="ERR_INVALID_RETURN_PROPERTY_VALUE"></a>
+### ERR_INVALID_RETURN_PROPERTY_VALUE
+
+Thrown in case a function option does not provide an expected value
+type for one of its returned object properties on execution.
+
 <a id="ERR_INVALID_RETURN_VALUE"></a>
 ### ERR_INVALID_RETURN_VALUE
 
-Thrown in case a function option does not return an expected value on execution.
+Thrown in case a function option does not return an expected value
+type on execution.
 For example when a function is expected to return a promise.
 
 <a id="ERR_INVALID_SYNC_FORK_INPUT"></a>

lib/internal/errors.js

@@ -681,6 +681,20 @@ E('ERR_INVALID_PROTOCOL',
  TypeError);
 E('ERR_INVALID_REPL_EVAL_CONFIG',
  'Cannot specify both "breakEvalOnSigint" and "eval" for REPL', TypeError);
+E('ERR_INVALID_RETURN_PROPERTY', (input, name, prop, value) =>{
+ return `Expected a valid ${input} to be returned for the "${prop}" from the` +
+ ` "${name}" function but got ${value}.`;
+}, TypeError);
+E('ERR_INVALID_RETURN_PROPERTY_VALUE', (input, name, prop, value) =>{
+ let type;
+ if (value && value.constructor && value.constructor.name){
+ type = `instance of ${value.constructor.name}`;
+ } else{
+ type = `type ${typeof value}`;
+ }
+ return `Expected ${input} to be returned for the "${prop}" from the` +
+ ` "${name}" function but got ${type}.`;
+}, TypeError);
 E('ERR_INVALID_RETURN_VALUE', (input, name, value) =>{
  let type;
  if (value && value.constructor && value.constructor.name){
@@ -689,7 +703,7 @@ E('ERR_INVALID_RETURN_VALUE', (input, name, value) =>{
  type = `type ${typeof value}`;
  }
  return `Expected ${input} to be returned from the "${name}"` +
-` function but got ${type}.`;
+ ` function but got ${type}.`;
 }, TypeError);
 E('ERR_INVALID_SYNC_FORK_INPUT',
  'Asynchronous forks do not support Buffer, Uint8Array or string input: %s',

lib/internal/modules/esm/loader.js

@@ -2,10 +2,13 @@
 
 const{
  ERR_INVALID_ARG_TYPE,
- ERR_INVALID_PROTOCOL,
+ ERR_INVALID_RETURN_PROPERTY,
+ ERR_INVALID_RETURN_PROPERTY_VALUE,
+ ERR_INVALID_RETURN_VALUE,
  ERR_MISSING_DYNAMIC_INTSTANTIATE_HOOK,
  ERR_UNKNOWN_MODULE_FORMAT
 } = require('internal/errors').codes;
+const{URL } = require('url');
 const ModuleMap = require('internal/modules/esm/module_map');
 const ModuleJob = require('internal/modules/esm/module_job');
 const defaultResolve = require('internal/modules/esm/default_resolve');
@@ -52,20 +55,42 @@ class Loader{
  if (!isMain && typeof parentURL !== 'string')
  throw new ERR_INVALID_ARG_TYPE('parentURL', 'string', parentURL);
 
- const{url, format } =
- await this._resolve(specifier, parentURL, defaultResolve);
+ const resolved = await this._resolve(specifier, parentURL, defaultResolve);
+
+ if (typeof resolved !== 'object')
+ throw new ERR_INVALID_RETURN_VALUE(
+ 'object', 'loader resolve', resolved
+ );
+
+ const{url, format } = resolved;
 
  if (typeof url !== 'string')
- throw new ERR_INVALID_ARG_TYPE('url', 'string', url);
+ throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
+ 'string', 'loader resolve', 'url', url
+ );
 
  if (typeof format !== 'string')
- throw new ERR_INVALID_ARG_TYPE('format', 'string', format);
+ throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
+ 'string', 'loader resolve', 'format', format
+ );
 
  if (format === 'builtin')
  return{url: `node:${url}`, format };
 
+ if (this._resolve !== defaultResolve){
+ try{
+ new URL(url);
+ } catch (e){
+ throw new ERR_INVALID_RETURN_PROPERTY(
+ 'url', 'loader resolve', 'url', url
+ );
+ }
+ }
+
  if (format !== 'dynamic' && !url.startsWith('file:'))
- throw new ERR_INVALID_PROTOCOL(url, 'file:');
+ throw new ERR_INVALID_RETURN_PROPERTY(
+ 'file: url', 'loader resolve', 'url', url
+ );
 
  return{url, format };
  }

test/common/index.mjs

@@ -1,71 +1,123 @@
 // Flags: --experimental-modules
 /* eslint-disable node-core/required-modules */
+importcommonfrom'./index.js';
 
-importassertfrom'assert';
+const{
+PORT,
+ isMainThread,
+ isWindows,
+ isWOW64,
+ isAIX,
+ isLinuxPPCBE,
+ isSunOS,
+ isFreeBSD,
+ isOpenBSD,
+ isLinux,
+ isOSX,
+ isGlibc,
+ enoughTestMem,
+ enoughTestCpu,
+ rootDir,
+ buildType,
+ localIPv6Hosts,
+ opensslCli,
+PIPE,
+ hasIPv6,
+ childShouldThrowAndAbort,
+ ddCommand,
+ spawnPwd,
+ spawnSyncPwd,
+ platformTimeout,
+ allowGlobals,
+ leakedGlobals,
+ mustCall,
+ mustCallAtLeast,
+ mustCallAsync,
+ hasMultiLocalhost,
+ fileExists,
+ skipIfEslintMissing,
+ canCreateSymLink,
+ getCallSite,
+ mustNotCall,
+ printSkipMessage,
+ skip,
+ ArrayStream,
+ nodeProcessAborted,
+ busyLoop,
+ isAlive,
+ noWarnCode,
+ expectWarning,
+ expectsError,
+ skipIfInspectorDisabled,
+ skipIf32Bits,
+ getArrayBufferViews,
+ getBufferSources,
+ crashOnUnhandledRejection,
+ getTTYfd,
+ runWithInvalidFD,
+ hijackStdout,
+ hijackStderr,
+ restoreStdout,
+ restoreStderr,
+ isCPPSymbolsNotMapped
+}=common;
 
-letknownGlobals=[
-Buffer,
-clearImmediate,
-clearInterval,
-clearTimeout,
-global,
-process,
-setImmediate,
-setInterval,
-setTimeout
-];
-
-if(process.env.NODE_TEST_KNOWN_GLOBALS){
-constknownFromEnv=process.env.NODE_TEST_KNOWN_GLOBALS.split(',');
-allowGlobals(...knownFromEnv);
-}
-
-exportfunctionallowGlobals(...whitelist){
-knownGlobals=knownGlobals.concat(whitelist);
-}
-
-exportfunctionleakedGlobals(){
-// Add possible expected globals
-if(global.gc){
-knownGlobals.push(global.gc);
-}
-
-if(global.DTRACE_HTTP_SERVER_RESPONSE){
-knownGlobals.push(DTRACE_HTTP_SERVER_RESPONSE);
-knownGlobals.push(DTRACE_HTTP_SERVER_REQUEST);
-knownGlobals.push(DTRACE_HTTP_CLIENT_RESPONSE);
-knownGlobals.push(DTRACE_HTTP_CLIENT_REQUEST);
-knownGlobals.push(DTRACE_NET_STREAM_END);
-knownGlobals.push(DTRACE_NET_SERVER_CONNECTION);
-}
-
-if(global.COUNTER_NET_SERVER_CONNECTION){
-knownGlobals.push(COUNTER_NET_SERVER_CONNECTION);
-knownGlobals.push(COUNTER_NET_SERVER_CONNECTION_CLOSE);
-knownGlobals.push(COUNTER_HTTP_SERVER_REQUEST);
-knownGlobals.push(COUNTER_HTTP_SERVER_RESPONSE);
-knownGlobals.push(COUNTER_HTTP_CLIENT_REQUEST);
-knownGlobals.push(COUNTER_HTTP_CLIENT_RESPONSE);
-}
-
-constleaked=[];
-
-for(constvalinglobal){
-if(!knownGlobals.includes(global[val])){
-leaked.push(val);
-}
-}
-
-if(global.__coverage__){
-returnleaked.filter((varname)=>!/^(?:cov_|__cov)/.test(varname));
-}else{
-returnleaked;
-}
-}
-
-process.on('exit',function(){
-constleaked=leakedGlobals();
-if(leaked.length>0){
-assert.fail(`Unexpected global(s) found: ${leaked.join(', ')}`);
-}
-});
+export{
+PORT,
+isMainThread,
+isWindows,
+isWOW64,
+isAIX,
+isLinuxPPCBE,
+isSunOS,
+isFreeBSD,
+isOpenBSD,
+isLinux,
+isOSX,
+isGlibc,
+enoughTestMem,
+enoughTestCpu,
+rootDir,
+buildType,
+localIPv6Hosts,
+opensslCli,
+PIPE,
+hasIPv6,
+childShouldThrowAndAbort,
+ddCommand,
+spawnPwd,
+spawnSyncPwd,
+platformTimeout,
+allowGlobals,
+leakedGlobals,
+mustCall,
+mustCallAtLeast,
+mustCallAsync,
+hasMultiLocalhost,
+fileExists,
+skipIfEslintMissing,
+canCreateSymLink,
+getCallSite,
+mustNotCall,
+printSkipMessage,
+skip,
+ArrayStream,
+nodeProcessAborted,
+busyLoop,
+isAlive,
+noWarnCode,
+expectWarning,
+expectsError,
+skipIfInspectorDisabled,
+skipIf32Bits,
+getArrayBufferViews,
+getBufferSources,
+crashOnUnhandledRejection,
+getTTYfd,
+runWithInvalidFD,
+hijackStdout,
+hijackStderr,
+restoreStdout,
+restoreStderr,
+isCPPSymbolsNotMapped
+};

test/es-module/test-esm-loader-invalid-format.mjs

@@ -0,0 +1,11 @@
+// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-format.mjs
+import{expectsError, mustCall } from '../common'
+import assert from 'assert'
+
+import('../fixtures/es-modules/test-esm-ok.mjs')
+.then(assert.fail, expectsError({
+ code: 'ERR_INVALID_RETURN_PROPERTY_VALUE',
+ message: 'Expected string to be returned for the "format" from the ' +
+ '"loader resolve" function but got type undefined.'
+}))
+.then(mustCall());

test/es-module/test-esm-loader-invalid-url.mjs

@@ -0,0 +1,12 @@
+// Flags: --experimental-modules --loader ./test/fixtures/es-module-loaders/loader-invalid-url.mjs
+import{expectsError, mustCall } from '../common'
+import assert from 'assert'
+
+import('../fixtures/es-modules/test-esm-ok.mjs')
+.then(assert.fail, expectsError({
+ code: 'ERR_INVALID_RETURN_PROPERTY',
+ message: 'Expected a valid url to be returned for the "url" from the ' +
+ '"loader resolve" function but got ' +
+ '../fixtures/es-modules/test-esm-ok.mjs.'
+}))
+.then(mustCall());

test/fixtures/es-module-loaders/loader-invalid-format.mjs

@@ -0,0 +1,8 @@
+export async function resolve(specifier, parentModuleURL, defaultResolve){
+ if (parentModuleURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs'){
+ return{
+ url: 'file:///asdf'
+ };
+ }
+ return defaultResolve(specifier, parentModuleURL);
+}

test/fixtures/es-module-loaders/loader-invalid-url.mjs

@@ -0,0 +1,9 @@
+export async function resolve(specifier, parentModuleURL, defaultResolve){
+ if (parentModuleURL && specifier === '../fixtures/es-modules/test-esm-ok.mjs'){
+ return{
+ url: specifier,
+ format: 'esm'
+ };
+ }
+ return defaultResolve(specifier, parentModuleURL);
+}