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>

5 files changed, 159 insertions, 67 deletions

diff --git a/test/common/index.mjs b/test/common/index.mjs
index b7ac556bca..f73a9d9be5 100644
--- a/test/common/index.mjs
+++ b/test/common/index.mjs
@@ -1,71 +1,123 @@
 // Flags: --experimental-modules
 /* eslint-disable node-core/required-modules */
+import common from './index.js';
 
-import assert from '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;
 
-let knownGlobals = [
-  Buffer,
-  clearImmediate,
-  clearInterval,
-  clearTimeout,
-  global,
-  process,
-  setImmediate,
-  setInterval,
-  setTimeout
-];
-
-if (process.env.NODE_TEST_KNOWN_GLOBALS) {
-  const knownFromEnv = process.env.NODE_TEST_KNOWN_GLOBALS.split(',');
-  allowGlobals(...knownFromEnv);
-}
-
-export function allowGlobals(...whitelist) {
-  knownGlobals = knownGlobals.concat(whitelist);
-}
-
-export function leakedGlobals() {
-  // 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);
-  }
-
-  const leaked = [];
-
-  for (const val in global) {
-    if (!knownGlobals.includes(global[val])) {
-      leaked.push(val);
-    }
-  }
-
-  if (global.__coverage__) {
-    return leaked.filter((varname) => !/^(?:cov_|__cov)/.test(varname));
-  } else {
-    return leaked;
-  }
-}
-
-process.on('exit', function() {
-  const leaked = 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
+};
diff --git a/test/es-module/test-esm-loader-invalid-format.mjs b/test/es-module/test-esm-loader-invalid-format.mjs
new file mode 100644
index 0000000000..88b128affd
--- /dev/null
+++ b/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',
+  message: 'Expected string to be returned for the "url" from the ' +
+           '"loader resolve" function but got "undefined"'
+}))
+.then(mustCall());
diff --git a/test/es-module/test-esm-loader-invalid-url.mjs b/test/es-module/test-esm-loader-invalid-url.mjs
new file mode 100644
index 0000000000..43971a2e6e
--- /dev/null
+++ b/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());
diff --git a/test/fixtures/es-module-loaders/loader-invalid-format.mjs b/test/fixtures/es-module-loaders/loader-invalid-format.mjs
new file mode 100644
index 0000000000..17a0dcd04d
--- /dev/null
+++ b/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);
+}
diff --git a/test/fixtures/es-module-loaders/loader-invalid-url.mjs b/test/fixtures/es-module-loaders/loader-invalid-url.mjs
new file mode 100644
index 0000000000..12efbb5021
--- /dev/null
+++ b/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);
+}