Skip to content

Commit 889add6

Browse files
anonrigtargos
authored andcommitted
esm: avoid try/catch when validating urls
PR-URL: #47541 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Stephen Belanger <[email protected]>
1 parent b0b16ee commit 889add6

File tree

2 files changed

+12
-16
lines changed

2 files changed

+12
-16
lines changed

lib/internal/modules/esm/hooks.js

+9-8
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ const {
3535
} = require('internal/errors').codes;
3636
const { exitCodes: { kUnfinishedTopLevelAwait } } = internalBinding('errors');
3737
const { URL } = require('internal/url');
38+
const { canParse: urlCanParse } = internalBinding('url');
3839
const { receiveMessageOnPort } = require('worker_threads');
3940
const {
4041
isAnyArrayBuffer,
@@ -272,17 +273,17 @@ class Hooks {
272273

273274
// Avoid expensive URL instantiation for known-good URLs
274275
if (!this.#validatedUrls.has(url)) {
275-
try {
276-
new URL(url);
277-
this.#validatedUrls.add(url);
278-
} catch {
276+
// No need to convert to string, since the type is already validated
277+
if (!urlCanParse(url)) {
279278
throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
280279
'a URL string',
281280
hookErrIdentifier,
282281
'url',
283282
url,
284283
);
285284
}
285+
286+
this.#validatedUrls.add(url);
286287
}
287288

288289
if (
@@ -352,16 +353,16 @@ class Hooks {
352353

353354
// Avoid expensive URL instantiation for known-good URLs
354355
if (!this.#validatedUrls.has(nextUrl)) {
355-
try {
356-
new URL(nextUrl);
357-
this.#validatedUrls.add(nextUrl);
358-
} catch {
356+
// No need to convert to string, since the type is already validated
357+
if (!urlCanParse(nextUrl)) {
359358
throw new ERR_INVALID_ARG_VALUE(
360359
`${hookErrIdentifier} url`,
361360
nextUrl,
362361
'should be a URL string',
363362
);
364363
}
364+
365+
this.#validatedUrls.add(nextUrl);
365366
}
366367

367368
if (ctx) { validateObject(ctx, `${hookErrIdentifier} context`); }

lib/internal/modules/esm/resolve.js

+3-8
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ const experimentalNetworkImports =
4242
getOptionValue('--experimental-network-imports');
4343
const typeFlag = getOptionValue('--input-type');
4444
const { URL, pathToFileURL, fileURLToPath, isURL } = require('internal/url');
45+
const { canParse: canParseURL } = internalBinding('url');
4546
const {
4647
ERR_INPUT_TYPE_NOT_ALLOWED,
4748
ERR_INVALID_ARG_TYPE,
@@ -333,14 +334,8 @@ function resolvePackageTargetString(
333334
if (!StringPrototypeStartsWith(target, './')) {
334335
if (internal && !StringPrototypeStartsWith(target, '../') &&
335336
!StringPrototypeStartsWith(target, '/')) {
336-
let isURL = false;
337-
try {
338-
new URL(target);
339-
isURL = true;
340-
} catch {
341-
// Continue regardless of error.
342-
}
343-
if (!isURL) {
337+
// No need to convert target to string, since it's already presumed to be
338+
if (!canParseURL(target)) {
344339
const exportTarget = pattern ?
345340
RegExpPrototypeSymbolReplace(patternRegEx, target, () => subpath) :
346341
target + subpath;

0 commit comments

Comments
 (0)