From 9cfdd53bb4d828ea68f3a0f9fa34b0a88aac9c06 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Tue, 23 Apr 2024 22:43:05 -0400 Subject: [PATCH] lib: reduce amount of caught URL errors --- lib/internal/modules/esm/hooks.js | 6 +--- lib/internal/modules/esm/resolve.js | 33 ++++++++------------- lib/internal/modules/esm/translators.js | 15 ++++++---- lib/internal/source_map/source_map_cache.js | 13 ++++---- lib/internal/url.js | 14 ++++----- 5 files changed, 34 insertions(+), 47 deletions(-) diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index ba655116a0bb572..e35da6c6b8676b4 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -403,11 +403,7 @@ class Hooks { let responseURLObj; if (typeof responseURL === 'string') { - try { - responseURLObj = new URL(responseURL); - } catch { - // responseURLObj not defined will throw in next branch. - } + responseURLObj = URL.parse(responseURL); } if (responseURLObj?.href !== responseURL) { diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index bbbaed874792898..a2bc7937e127538 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -37,7 +37,7 @@ const experimentalNetworkImports = getOptionValue('--experimental-network-imports'); const inputTypeFlag = getOptionValue('--input-type'); const { URL, pathToFileURL, fileURLToPath, isURL } = require('internal/url'); -const { getCWDURL, setOwnProperty } = require('internal/util'); +const { getCWDURL } = require('internal/util'); const { canParse: URLCanParse } = internalBinding('url'); const { legacyMainResolve: FSLegacyMainResolve } = internalBinding('fs'); const { @@ -897,26 +897,21 @@ function moduleResolve(specifier, base, conditions, preserveSymlinks) { // Ok since relative URLs cannot parse as URLs. let resolved; if (shouldBeTreatedAsRelativeOrAbsolutePath(specifier)) { - try { - resolved = new URL(specifier, base); - } catch (cause) { - const error = new ERR_UNSUPPORTED_RESOLVE_REQUEST(specifier, base); - setOwnProperty(error, 'cause', cause); - throw error; + resolved = URL.parse(specifier, base); + + if (resolved == null) { + throw new ERR_UNSUPPORTED_RESOLVE_REQUEST(specifier, base); } } else if (protocol === 'file:' && specifier[0] === '#') { resolved = packageImportsResolve(specifier, base, conditions); } else { - try { - resolved = new URL(specifier); - } catch (cause) { - if (isRemote && !BuiltinModule.canBeRequiredWithoutScheme(specifier)) { - const error = new ERR_UNSUPPORTED_RESOLVE_REQUEST(specifier, base); - setOwnProperty(error, 'cause', cause); - throw error; - } - resolved = packageResolve(specifier, base, conditions); + resolved = URL.parse(specifier); + + if (resolved == null && isRemote && !BuiltinModule.canBeRequiredWithoutScheme(specifier)) { + throw new ERR_UNSUPPORTED_RESOLVE_REQUEST(specifier, base); } + + resolved ??= packageResolve(specifier, base, conditions); } if (resolved.protocol !== 'file:') { return resolved; @@ -1082,11 +1077,7 @@ function defaultResolve(specifier, context = {}) { let parsedParentURL; if (parentURL) { - try { - parsedParentURL = new URL(parentURL); - } catch { - // Ignore exception - } + parsedParentURL = URL.parse(parentURL); } let parsed, protocol; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 8f4b6b25d888968..f0a7cf50aaa0153 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -350,12 +350,17 @@ translators.set('commonjs', async function commonjsStrategy(url, source, } } : loadCJSModule; - try { - // We still need to read the FS to detect the exports. - source ??= readFileSync(new URL(url), 'utf8'); - } catch { - // Continue regardless of error. + const urlPath = URL.parse(url); + + if (urlPath != null) { + try { + // We still need to read the FS to detect the exports. + source ??= readFileSync(urlPath, 'utf8'); + } catch { + // Continue regardless of error. + } } + return createCJSModuleWrap(url, source, isMain, cjsLoader); }); diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index 53c3374fc09176e..683fc5afefcf5f0 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -187,8 +187,9 @@ function maybeCacheGeneratedSourceMap(content) { } function dataFromUrl(sourceURL, sourceMappingURL) { - try { - const url = new URL(sourceMappingURL); + const url = URL.parse(sourceMappingURL); + + if (url != null) { switch (url.protocol) { case 'data:': return sourceMapFromDataUrl(sourceURL, url.pathname); @@ -196,12 +197,10 @@ function dataFromUrl(sourceURL, sourceMappingURL) { debug(`unknown protocol ${url.protocol}`); return null; } - } catch (err) { - debug(err); - // If no scheme is present, we assume we are dealing with a file path. - const mapURL = new URL(sourceMappingURL, sourceURL).href; - return sourceMapFromFile(mapURL); } + + const mapURL = new URL(sourceMappingURL, sourceURL).href; + return sourceMapFromFile(mapURL); } // Cache the length of each line in the file that a source map was extracted diff --git a/lib/internal/url.js b/lib/internal/url.js index 39d79392bd86bc0..31562668b0cd81b 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -958,15 +958,11 @@ class URL { if (protocol === 'blob:') { const path = this.pathname; if (path.length > 0) { - try { - const out = new URL(path); - // Only return origin of scheme is `http` or `https` - // Otherwise return a new opaque origin (null). - if (out.#context.scheme_type === 0 || out.#context.scheme_type === 2) { - return `${out.protocol}//${out.host}`; - } - } catch { - // Do nothing. + const out = URL.parse(path); + // Only return origin of scheme is `http` or `https` + // Otherwise return a new opaque origin (null). + if (out && (out.#context.scheme_type === 0 || out.#context.scheme_type === 2)) { + return `${out.protocol}//${out.host}`; } } }