From c10ee5d5aa6aaa2f48102e58e5a9edf44e42997f Mon Sep 17 00:00:00 2001
From: Thomas <hakerh403@gmail.com>
Date: Sun, 29 Dec 2019 13:56:43 +0100
Subject: [PATCH] esm: better error message for unsupported URL

The default ESM loader supports only file and data URLs.
This PR adds better error message for it.

PR-URL: https://github.com/nodejs/node/pull/31129
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
---
 doc/api/errors.md                           |  5 +++++
 lib/internal/errors.js                      |  2 ++
 lib/internal/modules/esm/default_resolve.js |  8 ++++++--
 test/es-module/test-esm-dynamic-import.js   | 16 ++++++++--------
 4 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/doc/api/errors.md b/doc/api/errors.md
index 01bd6676d89b91..cb5444bea50a2b 100644
--- a/doc/api/errors.md
+++ b/doc/api/errors.md
@@ -1936,6 +1936,11 @@ An attempt was made to load a module with an unknown or unsupported format.
 An invalid or unknown process signal was passed to an API expecting a valid
 signal (such as [`subprocess.kill()`][]).
 
+<a id="ERR_UNSUPPORTED_ESM_URL_SCHEME"></a>
+### `ERR_UNSUPPORTED_ESM_URL_SCHEME`
+
+`import` with URL schemes other than `file` and `data` is unsupported.
+
 <a id="ERR_V8BREAKITERATOR"></a>
 ### `ERR_V8BREAKITERATOR`
 
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index fcc9ee72ea27c8..cee613e76d7f35 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -1218,6 +1218,8 @@ E('ERR_UNKNOWN_FILE_EXTENSION',
   TypeError);
 E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s', RangeError);
 E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError);
+E('ERR_UNSUPPORTED_ESM_URL_SCHEME', 'Only file and data URLs are supported ' +
+  'by the default ESM loader', Error);
 
 E('ERR_V8BREAKITERATOR',
   'Full ICU data not installed. See https://github.com/nodejs/node/wiki/Intl',
diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js
index bd7f6421a8b579..ab751da5eab512 100644
--- a/lib/internal/modules/esm/default_resolve.js
+++ b/lib/internal/modules/esm/default_resolve.js
@@ -17,7 +17,8 @@ const { resolve: moduleWrapResolve,
         getPackageType } = internalBinding('module_wrap');
 const { URL, pathToFileURL, fileURLToPath } = require('internal/url');
 const { ERR_INPUT_TYPE_NOT_ALLOWED,
-        ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes;
+        ERR_UNKNOWN_FILE_EXTENSION,
+        ERR_UNSUPPORTED_ESM_URL_SCHEME } = require('internal/errors').codes;
 
 const { SafeMap } = primordials;
 
@@ -50,8 +51,9 @@ if (experimentalJsonModules)
   extensionFormatMap['.json'] = legacyExtensionFormatMap['.json'] = 'json';
 
 function resolve(specifier, parentURL) {
+  let parsed;
   try {
-    const parsed = new URL(specifier);
+    parsed = new URL(specifier);
     if (parsed.protocol === 'data:') {
       const [ , mime ] = /^([^/]+\/[^;,]+)(?:[^,]*?)(;base64)?,/.exec(parsed.pathname) || [ null, null, null ];
       const format = ({
@@ -66,6 +68,8 @@ function resolve(specifier, parentURL) {
       };
     }
   } catch {}
+  if (parsed && parsed.protocol !== 'file:' && parsed.protocol !== 'data:')
+    throw new ERR_UNSUPPORTED_ESM_URL_SCHEME();
   if (NativeModule.canBeRequiredByUsers(specifier)) {
     return {
       url: specifier,
diff --git a/test/es-module/test-esm-dynamic-import.js b/test/es-module/test-esm-dynamic-import.js
index ca9c99007b5b1b..d80daf878411fd 100644
--- a/test/es-module/test-esm-dynamic-import.js
+++ b/test/es-module/test-esm-dynamic-import.js
@@ -17,8 +17,8 @@ function expectErrorProperty(result, propertyKey, value) {
     }));
 }
 
-function expectMissingModuleError(result) {
-  expectErrorProperty(result, 'code', 'ERR_MODULE_NOT_FOUND');
+function expectModuleError(result, err) {
+  expectErrorProperty(result, 'code', err);
 }
 
 function expectOkNamespace(result) {
@@ -55,10 +55,10 @@ function expectFsNamespace(result) {
   expectFsNamespace(eval('import("fs")'));
   expectFsNamespace(eval('import("fs")'));
 
-  expectMissingModuleError(import('./not-an-existing-module.mjs'));
-  // TODO(jkrems): Right now this doesn't hit a protocol error because the
-  // module resolution step already rejects it. These arguably should be
-  // protocol errors.
-  expectMissingModuleError(import('node:fs'));
-  expectMissingModuleError(import('http://example.com/foo.js'));
+  expectModuleError(import('./not-an-existing-module.mjs'),
+                    'ERR_MODULE_NOT_FOUND');
+  expectModuleError(import('node:fs'),
+                    'ERR_UNSUPPORTED_ESM_URL_SCHEME');
+  expectModuleError(import('http://example.com/foo.js'),
+                    'ERR_UNSUPPORTED_ESM_URL_SCHEME');
 })();