From 08832e76d3502d16389e02d9f26db1a66bc5b2a2 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 31 Jan 2024 23:22:18 +0100 Subject: [PATCH] esm: improve error when calling `import.meta.resolve` from `data:` URL PR-URL: https://github.com/nodejs/node/pull/49516 Reviewed-By: James M Snell --- doc/api/errors.md | 22 +++++++++++ lib/internal/errors.js | 3 ++ lib/internal/modules/esm/resolve.js | 39 +++++++++++++------ .../test-esm-import-meta-resolve.mjs | 22 +++++++++++ 4 files changed, 75 insertions(+), 11 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index c11053981687f9..e9ce687b4f7581 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2956,6 +2956,26 @@ import 'package-name'; // supported `import` with URL schemes other than `file` and `data` is unsupported. + + +### `ERR_UNSUPPORTED_RESOLVE_REQUEST` + +An attempt was made to resolve an invalid module referrer. This can happen when +importing or calling `import.meta.resolve()` with either: + +* a bare specifier that is not a builtin module from a module whose URL scheme + is not `file`. +* a [relative URL][] from a module whose URL scheme is not a [special scheme][]. + +```mjs +try { + // Trying to import the package 'bare-specifier' from a `data:` URL module: + await import('data:text/javascript,import "bare-specifier"'); +} catch (e) { + console.log(e.code); // ERR_UNSUPPORTED_RESOLVE_REQUEST +} +``` + ### `ERR_USE_AFTER_CLOSE` @@ -3719,7 +3739,9 @@ The native call from `process.cpuUsage` could not be processed. [event emitter-based]: events.md#class-eventemitter [file descriptors]: https://en.wikipedia.org/wiki/File_descriptor [policy]: permissions.md#policies +[relative URL]: https://url.spec.whatwg.org/#relative-url-string [self-reference a package using its name]: packages.md#self-referencing-a-package-using-its-name +[special scheme]: https://url.spec.whatwg.org/#special-scheme [stream-based]: stream.md [syscall]: https://man7.org/linux/man-pages/man2/syscalls.2.html [try-catch]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/try...catch diff --git a/lib/internal/errors.js b/lib/internal/errors.js index d2250f5c7f053c..36d244120f0d56 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1863,6 +1863,9 @@ E('ERR_UNSUPPORTED_ESM_URL_SCHEME', (url, supported) => { msg += `. Received protocol '${url.protocol}'`; return msg; }, Error); +E('ERR_UNSUPPORTED_RESOLVE_REQUEST', + 'Failed to resolve module specifier "%s" from "%s": Invalid relative URL or base scheme is not hierarchical.', + TypeError); E('ERR_USE_AFTER_CLOSE', '%s was closed', Error); // This should probably be a `TypeError`. diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 54cdbe8e6175ad..bbbaed87479289 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -37,7 +37,7 @@ const experimentalNetworkImports = getOptionValue('--experimental-network-imports'); const inputTypeFlag = getOptionValue('--input-type'); const { URL, pathToFileURL, fileURLToPath, isURL } = require('internal/url'); -const { getCWDURL } = require('internal/util'); +const { getCWDURL, setOwnProperty } = require('internal/util'); const { canParse: URLCanParse } = internalBinding('url'); const { legacyMainResolve: FSLegacyMainResolve } = internalBinding('fs'); const { @@ -51,6 +51,7 @@ const { ERR_PACKAGE_IMPORT_NOT_DEFINED, ERR_PACKAGE_PATH_NOT_EXPORTED, ERR_UNSUPPORTED_DIR_IMPORT, + ERR_UNSUPPORTED_RESOLVE_REQUEST, ERR_NETWORK_IMPORT_DISALLOWED, } = require('internal/errors').codes; @@ -884,22 +885,37 @@ function shouldBeTreatedAsRelativeOrAbsolutePath(specifier) { * @param {boolean} preserveSymlinks - Whether to preserve symlinks in the resolved URL. */ function moduleResolve(specifier, base, conditions, preserveSymlinks) { - const isRemote = base.protocol === 'http:' || - base.protocol === 'https:'; + const protocol = typeof base === 'string' ? + StringPrototypeSlice(base, 0, StringPrototypeIndexOf(base, ':') + 1) : + base.protocol; + const isData = protocol === 'data:'; + const isRemote = + isData || + protocol === 'http:' || + protocol === 'https:'; // Order swapped from spec for minor perf gain. // Ok since relative URLs cannot parse as URLs. let resolved; if (shouldBeTreatedAsRelativeOrAbsolutePath(specifier)) { - resolved = new URL(specifier, base); - } else if (!isRemote && specifier[0] === '#') { + try { + resolved = new URL(specifier, base); + } catch (cause) { + const error = new ERR_UNSUPPORTED_RESOLVE_REQUEST(specifier, base); + setOwnProperty(error, 'cause', cause); + throw error; + } + } else if (protocol === 'file:' && specifier[0] === '#') { resolved = packageImportsResolve(specifier, base, conditions); } else { try { resolved = new URL(specifier); - } catch { - if (!isRemote) { - resolved = packageResolve(specifier, base, conditions); + } catch (cause) { + if (isRemote && !BuiltinModule.canBeRequiredWithoutScheme(specifier)) { + const error = new ERR_UNSUPPORTED_RESOLVE_REQUEST(specifier, base); + setOwnProperty(error, 'cause', cause); + throw error; } + resolved = packageResolve(specifier, base, conditions); } } if (resolved.protocol !== 'file:') { @@ -1073,7 +1089,7 @@ function defaultResolve(specifier, context = {}) { } } - let parsed; + let parsed, protocol; try { if (shouldBeTreatedAsRelativeOrAbsolutePath(specifier)) { parsed = new URL(specifier, parsedParentURL); @@ -1082,7 +1098,7 @@ function defaultResolve(specifier, context = {}) { } // Avoid accessing the `protocol` property due to the lazy getters. - const protocol = parsed.protocol; + protocol = parsed.protocol; if (protocol === 'data:' || (experimentalNetworkImports && ( @@ -1109,7 +1125,8 @@ function defaultResolve(specifier, context = {}) { if (maybeReturn) { return maybeReturn; } // This must come after checkIfDisallowedImport - if (parsed && parsed.protocol === 'node:') { return { __proto__: null, url: specifier }; } + protocol ??= parsed?.protocol; + if (protocol === 'node:') { return { __proto__: null, url: specifier }; } const isMain = parentURL === undefined; diff --git a/test/es-module/test-esm-import-meta-resolve.mjs b/test/es-module/test-esm-import-meta-resolve.mjs index a6435655750c88..4291f882753774 100644 --- a/test/es-module/test-esm-import-meta-resolve.mjs +++ b/test/es-module/test-esm-import-meta-resolve.mjs @@ -36,6 +36,28 @@ assert.strictEqual(import.meta.resolve('http://some-absolute/url'), 'http://some assert.strictEqual(import.meta.resolve('some://weird/protocol'), 'some://weird/protocol'); assert.strictEqual(import.meta.resolve('baz/', fixtures), fixtures + 'node_modules/baz/'); +assert.deepStrictEqual( + { ...await import('data:text/javascript,export default import.meta.resolve("http://some-absolute/url")') }, + { default: 'http://some-absolute/url' }, +); +assert.deepStrictEqual( + { ...await import('data:text/javascript,export default import.meta.resolve("some://weird/protocol")') }, + { default: 'some://weird/protocol' }, +); +assert.deepStrictEqual( + { ...await import(`data:text/javascript,export default import.meta.resolve("baz/", ${JSON.stringify(fixtures)})`) }, + { default: fixtures + 'node_modules/baz/' }, +); +assert.deepStrictEqual( + { ...await import('data:text/javascript,export default import.meta.resolve("fs")') }, + { default: 'node:fs' }, +); +await assert.rejects(import('data:text/javascript,export default import.meta.resolve("does-not-exist")'), { + code: 'ERR_UNSUPPORTED_RESOLVE_REQUEST', +}); +await assert.rejects(import('data:text/javascript,export default import.meta.resolve("./relative")'), { + code: 'ERR_UNSUPPORTED_RESOLVE_REQUEST', +}); { const cp = spawn(execPath, [