Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 889add6

Browse filesBrowse files
anonrigtargos
authored andcommitted
esm: avoid try/catch when validating urls
PR-URL: #47541 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent b0b16ee commit 889add6
Copy full SHA for 889add6

File tree

Expand file treeCollapse file tree

2 files changed

+12
-16
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+12
-16
lines changed
Open diff view settings
Collapse file

‎lib/internal/modules/esm/hooks.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/hooks.js
+9-8Lines changed: 9 additions & 8 deletions
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`); }
Collapse file

‎lib/internal/modules/esm/resolve.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/resolve.js
+3-8Lines changed: 3 additions & 8 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.