esm: avoid try/catch when validating urls

PR-URL: nodejs/node#47541 Backport-PR-URL: nodejs/node#50669 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>
sercher · Apr 25, 2024 · 9c2429d · 9c2429d
1 parent 33191fd
commit 9c2429d
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 16 deletions.
diff --git a/graal-nodejs/lib/internal/modules/esm/hooks.js b/graal-nodejs/lib/internal/modules/esm/hooks.js
@@ -34,6 +34,7 @@ const {
   ERR_WORKER_UNSERIALIZABLE_ERROR,
 } = require('internal/errors').codes;
 const { URL } = require('internal/url');
+const { canParse: urlCanParse } = internalBinding('url');
 const { receiveMessageOnPort } = require('worker_threads');
 const {
   isAnyArrayBuffer,
@@ -270,17 +271,17 @@ class Hooks {
 
     // Avoid expensive URL instantiation for known-good URLs
     if (!this.#validatedUrls.has(url)) {
-      try {
-        new URL(url);
-        this.#validatedUrls.add(url);
-      } catch {
+      // No need to convert to string, since the type is already validated
+      if (!urlCanParse(url)) {
         throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
           'a URL string',
           hookErrIdentifier,
           'url',
           url,
         );
       }
+
+      this.#validatedUrls.add(url);
     }
 
     if (
@@ -349,16 +350,16 @@ class Hooks {
 
       // Avoid expensive URL instantiation for known-good URLs
       if (!this.#validatedUrls.has(nextUrl)) {
-        try {
-          new URL(nextUrl);
-          this.#validatedUrls.add(nextUrl);
-        } catch {
+        // No need to convert to string, since the type is already validated
+        if (!urlCanParse(nextUrl)) {
           throw new ERR_INVALID_ARG_VALUE(
             `${hookErrIdentifier} url`,
             nextUrl,
             'should be a URL string',
           );
         }
+
+        this.#validatedUrls.add(nextUrl);
       }
 
       if (ctx) { validateObject(ctx, `${hookErrIdentifier} context`); }

diff --git a/graal-nodejs/lib/internal/modules/esm/resolve.js b/graal-nodejs/lib/internal/modules/esm/resolve.js
@@ -39,6 +39,7 @@ const experimentalNetworkImports =
   getOptionValue('--experimental-network-imports');
 const typeFlag = getOptionValue('--input-type');
 const { URL, pathToFileURL, fileURLToPath, toPathIfFileURL, isURL } = require('internal/url');
+const { canParse: canParseURL } = internalBinding('url');
 const {
   ERR_INPUT_TYPE_NOT_ALLOWED,
   ERR_INVALID_ARG_TYPE,
@@ -393,14 +394,8 @@ function resolvePackageTargetString(
   if (!StringPrototypeStartsWith(target, './')) {
     if (internal && !StringPrototypeStartsWith(target, '../') &&
         !StringPrototypeStartsWith(target, '/')) {
-      let isURL = false;
-      try {
-        new URL(target);
-        isURL = true;
-      } catch {
-        // Continue regardless of error.
-      }
-      if (!isURL) {
+      // No need to convert target to string, since it's already presumed to be
+      if (!canParseURL(target)) {
         const exportTarget = pattern ?
           RegExpPrototypeSymbolReplace(patternRegEx, target, () => subpath) :
           target + subpath;