Skip to content

Commit

Permalink
lib: reduce amount of caught URL errors
Browse files Browse the repository at this point in the history
  • Loading branch information
anonrig committed May 15, 2024
1 parent 9807ede commit a69d20c
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 49 deletions.
8 changes: 2 additions & 6 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const {
ERR_WORKER_UNSERIALIZABLE_ERROR,
} = require('internal/errors').codes;
const { exitCodes: { kUnsettledTopLevelAwait } } = internalBinding('errors');
const { URL } = require('internal/url');
const { URL, URLParse } = require('internal/url');

Check failure on line 36 in lib/internal/modules/esm/hooks.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

'URL' is assigned a value but never used
const { canParse: URLCanParse } = internalBinding('url');
const { receiveMessageOnPort, isMainThread } = require('worker_threads');
const {
Expand Down Expand Up @@ -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 = URLParse(responseURL);
}

if (responseURLObj?.href !== responseURL) {
Expand Down
8 changes: 4 additions & 4 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@ const {
ERR_UNKNOWN_MODULE_FORMAT,
} = require('internal/errors').codes;
const { getOptionValue } = require('internal/options');
const { isURL, pathToFileURL, URL } = require('internal/url');
const { isURL, pathToFileURL, URL, URLParse } = require('internal/url');

Check failure on line 32 in lib/internal/modules/esm/loader.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

'URL' is assigned a value but never used
const { emitExperimentalWarning, kEmptyObject } = require('internal/util');
const {
compileSourceTextModule,
getDefaultConditions,
} = require('internal/modules/esm/utils');
const { kImplicitAssertType } = require('internal/modules/esm/assert');
const { canParse } = internalBinding('url');
const { ModuleWrap, kEvaluating, kEvaluated } = internalBinding('module_wrap');
const {
urlToFilename,
Expand Down Expand Up @@ -323,8 +322,9 @@ class ModuleLoader {
getModuleJobForRequire(specifier, parentURL, importAttributes) {
assert(getOptionValue('--experimental-require-module'));

if (canParse(specifier)) {
const protocol = new URL(specifier).protocol;
const parsed = URLParse(specifier);
if (parsed != null) {
const protocol = parsed.protocol;
if (protocol === 'https:' || protocol === 'http:') {
throw new ERR_NETWORK_IMPORT_DISALLOWED(specifier, parentURL,
'ES modules cannot be loaded by require() from the network');
Expand Down
35 changes: 13 additions & 22 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
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 { URL, pathToFileURL, fileURLToPath, isURL, URLParse } = require('internal/url');

Check failure on line 36 in lib/internal/modules/esm/resolve.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

'URLParse' is assigned a value but never used
const { getCWDURL } = require('internal/util');
const { canParse: URLCanParse } = internalBinding('url');
const { legacyMainResolve: FSLegacyMainResolve } = internalBinding('fs');
const {
Expand Down Expand Up @@ -893,26 +893,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;
Expand Down Expand Up @@ -1053,11 +1048,7 @@ function defaultResolve(specifier, context = {}) {

let parsedParentURL;
if (parentURL) {
try {
parsedParentURL = new URL(parentURL);
} catch {
// Ignore exception
}
parsedParentURL = URL.parse(parentURL);
}

let parsed, protocol;
Expand Down
1 change: 1 addition & 0 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ translators.set('commonjs', async function commonjsStrategy(url, source,
} catch {
// Continue regardless of error.
}

return createCJSModuleWrap(url, source, isMain, cjsLoader);

});
Expand Down
15 changes: 7 additions & 8 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const kSourceMappingURLMagicComment = /\/[*/]#\s+sourceMappingURL=(?<sourceMappi
const kSourceURLMagicComment = /\/[*/]#\s+sourceURL=(?<sourceURL>[^\s]+)/g;

const { isAbsolute } = require('path');
const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
const { fileURLToPath, pathToFileURL, URL, URLParse } = require('internal/url');

let SourceMap;

Expand Down Expand Up @@ -187,21 +187,20 @@ function maybeCacheGeneratedSourceMap(content) {
}

function dataFromUrl(sourceURL, sourceMappingURL) {
try {
const url = new URL(sourceMappingURL);
const url = URLParse(sourceMappingURL);

if (url != null) {
switch (url.protocol) {
case 'data:':
return sourceMapFromDataUrl(sourceURL, url.pathname);
default:
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
Expand Down
15 changes: 6 additions & 9 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -959,15 +959,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?.#context.scheme_type === 0 || out?.#context.scheme_type === 2) {
return `${out.protocol}//${out.host}`;
}
}
}
Expand Down Expand Up @@ -1601,6 +1597,7 @@ module.exports = {
installObjectURLMethods,
URL,
URLSearchParams,
URLParse: URL.parse,
domainToASCII,
domainToUnicode,
urlToHttpOptions,
Expand Down

0 comments on commit a69d20c

Please sign in to comment.