GeezerVison is showing github.com — esm: avoid try/catch when validating urls · nodejs/node@2a528b7
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/modules/esm/hooks.js +9-8Lines changed: 9 additions & 8 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -34,6 +34,7 @@ const { | 34 | 34 | ERR_WORKER_UNSERIALIZABLE_ERROR, | 35 | 35 | } = require('internal/errors').codes; | 36 | 36 | const { URL } = require('internal/url'); | | 37 | +const { canParse: urlCanParse } = internalBinding('url'); | 37 | 38 | const { receiveMessageOnPort } = require('worker_threads'); | 38 | 39 | const { | 39 | 40 | isAnyArrayBuffer, | @@ -270,17 +271,17 @@ class Hooks { | 270 | 271 |
| 271 | 272 | // Avoid expensive URL instantiation for known-good URLs | 272 | 273 | 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)) { | 277 | 276 | throw new ERR_INVALID_RETURN_PROPERTY_VALUE( | 278 | 277 | 'a URL string', | 279 | 278 | hookErrIdentifier, | 280 | 279 | 'url', | 281 | 280 | url, | 282 | 281 | ); | 283 | 282 | } | | 283 | + | | 284 | + this.#validatedUrls.add(url); | 284 | 285 | } | 285 | 286 |
| 286 | 287 | if ( | @@ -349,16 +350,16 @@ class Hooks { | 349 | 350 |
| 350 | 351 | // Avoid expensive URL instantiation for known-good URLs | 351 | 352 | 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)) { | 356 | 355 | throw new ERR_INVALID_ARG_VALUE( | 357 | 356 | `${hookErrIdentifier} url`, | 358 | 357 | nextUrl, | 359 | 358 | 'should be a URL string', | 360 | 359 | ); | 361 | 360 | } | | 361 | + | | 362 | + this.#validatedUrls.add(nextUrl); | 362 | 363 | } | 363 | 364 |
| 364 | 365 | if (ctx) { validateObject(ctx, `${hookErrIdentifier} context`); } | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/modules/esm/resolve.js +3-8Lines changed: 3 additions & 8 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -39,6 +39,7 @@ const experimentalNetworkImports = | 39 | 39 | getOptionValue('--experimental-network-imports'); | 40 | 40 | const typeFlag = getOptionValue('--input-type'); | 41 | 41 | const { URL, pathToFileURL, fileURLToPath, toPathIfFileURL, isURL } = require('internal/url'); | | 42 | +const { canParse: canParseURL } = internalBinding('url'); | 42 | 43 | const { | 43 | 44 | ERR_INPUT_TYPE_NOT_ALLOWED, | 44 | 45 | ERR_INVALID_ARG_TYPE, | @@ -393,14 +394,8 @@ function resolvePackageTargetString( | 393 | 394 | if (!StringPrototypeStartsWith(target, './')) { | 394 | 395 | if (internal && !StringPrototypeStartsWith(target, '../') && | 395 | 396 | !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)) { | 404 | 399 | const exportTarget = pattern ? | 405 | 400 | RegExpPrototypeSymbolReplace(patternRegEx, target, () => subpath) : | 406 | 401 | target + subpath; | |
|