Skip to content

Commit

Permalink
esm: improve error when calling import.meta.resolve from data: URL
Browse files Browse the repository at this point in the history
PR-URL: nodejs#49516
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
aduh95 authored and marco-ippolito committed Feb 19, 2024
1 parent 548e6b5 commit 0041ca5
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 11 deletions.
22 changes: 22 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -2959,6 +2959,26 @@ import 'package-name'; // supported

`import` with URL schemes other than `file` and `data` is unsupported.

<a id="ERR_UNSUPPORTED_RESOLVE_REQUEST"></a>

### `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
}
```

<a id="ERR_USE_AFTER_CLOSE"></a>

### `ERR_USE_AFTER_CLOSE`
Expand Down Expand Up @@ -3710,7 +3730,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
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
39 changes: 28 additions & 11 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;

Expand Down Expand Up @@ -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:') {
Expand Down Expand Up @@ -1073,7 +1089,7 @@ function defaultResolve(specifier, context = {}) {
}
}

let parsed;
let parsed, protocol;
try {
if (shouldBeTreatedAsRelativeOrAbsolutePath(specifier)) {
parsed = new URL(specifier, parsedParentURL);
Expand All @@ -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 &&
(
Expand All @@ -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;
Expand Down
22 changes: 22 additions & 0 deletions test/es-module/test-esm-import-meta-resolve.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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, [
Expand Down

0 comments on commit 0041ca5

Please sign in to comment.