esm: loader hook URL validation and error messages

PR-URL: https://github.com/nodejs/node/pull/21352
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

2 files changed, 46 insertions, 7 deletions

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 24e1f34b25..be37c96d01 100644
--- a/lib/internal/errors.js
+++ b/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',
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
index b3023fcd4f..2fa10c7eab 100644
--- a/lib/internal/modules/esm/loader.js
+++ b/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 };
   }