Skip to content

Commit 2a528b7

Browse files
anonrigtargos
authored andcommitted
esm: avoid try/catch when validating urls
PR-URL: #47541 Backport-PR-URL: #50669 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 bac9b17 commit 2a528b7

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
@@ -34,6 +34,7 @@ const {
3434
ERR_WORKER_UNSERIALIZABLE_ERROR,
3535
} = require('internal/errors').codes;
3636
const { URL } = require('internal/url');
37+
const { canParse: urlCanParse } = internalBinding('url');
3738
const { receiveMessageOnPort } = require('worker_threads');
3839
const {
3940
isAnyArrayBuffer,
@@ -270,17 +271,17 @@ class Hooks {
270271

271272
// Avoid expensive URL instantiation for known-good URLs
272273
if (!this.#validatedUrls.has(url)) {
273-
try {
274-
new URL(url);
275-
this.#validatedUrls.add(url);
276-
} catch {
274+
// No need to convert to string, since the type is already validated
275+
if (!urlCanParse(url)) {
277276
throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
278277
'a URL string',
279278
hookErrIdentifier,
280279
'url',
281280
url,
282281
);
283282
}
283+
284+
this.#validatedUrls.add(url);
284285
}
285286

286287
if (
@@ -349,16 +350,16 @@ class Hooks {
349350

350351
// Avoid expensive URL instantiation for known-good URLs
351352
if (!this.#validatedUrls.has(nextUrl)) {
352-
try {
353-
new URL(nextUrl);
354-
this.#validatedUrls.add(nextUrl);
355-
} catch {
353+
// No need to convert to string, since the type is already validated
354+
if (!urlCanParse(nextUrl)) {
356355
throw new ERR_INVALID_ARG_VALUE(
357356
`${hookErrIdentifier} url`,
358357
nextUrl,
359358
'should be a URL string',
360359
);
361360
}
361+
362+
this.#validatedUrls.add(nextUrl);
362363
}
363364

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

lib/internal/modules/esm/resolve.js

+3-8
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ const experimentalNetworkImports =
3939
getOptionValue('--experimental-network-imports');
4040
const typeFlag = getOptionValue('--input-type');
4141
const { URL, pathToFileURL, fileURLToPath, toPathIfFileURL, isURL } = require('internal/url');
42+
const { canParse: canParseURL } = internalBinding('url');
4243
const {
4344
ERR_INPUT_TYPE_NOT_ALLOWED,
4445
ERR_INVALID_ARG_TYPE,
@@ -393,14 +394,8 @@ function resolvePackageTargetString(
393394
if (!StringPrototypeStartsWith(target, './')) {
394395
if (internal && !StringPrototypeStartsWith(target, '../') &&
395396
!StringPrototypeStartsWith(target, '/')) {
396-
let isURL = false;
397-
try {
398-
new URL(target);
399-
isURL = true;
400-
} catch {
401-
// Continue regardless of error.
402-
}
403-
if (!isURL) {
397+
// No need to convert target to string, since it's already presumed to be
398+
if (!canParseURL(target)) {
404399
const exportTarget = pattern ?
405400
RegExpPrototypeSymbolReplace(patternRegEx, target, () => subpath) :
406401
target + subpath;

0 commit comments

Comments
 (0)