From 166eb782f9d3a7737d23299cc78eed79ba59185b Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Fri, 25 Feb 2022 10:05:55 -0600 Subject: [PATCH] esm: fix base URL for network imports PR-URL: https://github.com/nodejs/node/pull/42131 Reviewed-By: Geoffrey Booth Reviewed-By: Guy Bedford Reviewed-By: Benjamin Gruenbaum --- lib/internal/modules/cjs/helpers.js | 5 ++ lib/internal/modules/cjs/loader.js | 6 ++- .../modules/esm/initialize_import_meta.js | 16 ++----- lib/internal/modules/esm/loader.js | 46 ++++++++++++++++++- lib/internal/modules/esm/module_job.js | 3 +- lib/internal/modules/esm/translators.js | 4 +- test/es-module/test-http-imports.mjs | 15 ++++++ 7 files changed, 79 insertions(+), 16 deletions(-) diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index 3d27a19a250162..3ae63b46195f75 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -193,6 +193,11 @@ function addBuiltinLibsToObject(object, dummyModuleName) { }); } +/** + * + * @param {string | URL} referrer + * @returns {string} + */ function normalizeReferrerURL(referrer) { if (typeof referrer === 'string' && path.isAbsolute(referrer)) { return pathToFileURL(referrer).href; diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index cfe2982bf22c24..b4902850c7fec5 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1023,7 +1023,8 @@ function wrapSafe(filename, content, cjsModuleInstance) { displayErrors: true, importModuleDynamically: async (specifier, _, importAssertions) => { const loader = asyncESM.esmLoader; - return loader.import(specifier, normalizeReferrerURL(filename), + return loader.import(specifier, + loader.getBaseURL(normalizeReferrerURL(filename)), importAssertions); }, }); @@ -1039,7 +1040,8 @@ function wrapSafe(filename, content, cjsModuleInstance) { filename, importModuleDynamically(specifier, _, importAssertions) { const loader = asyncESM.esmLoader; - return loader.import(specifier, normalizeReferrerURL(filename), + return loader.import(specifier, + loader.getBaseURL(normalizeReferrerURL(filename)), importAssertions); }, }); diff --git a/lib/internal/modules/esm/initialize_import_meta.js b/lib/internal/modules/esm/initialize_import_meta.js index cb9fa23f966f76..f1daabbb6425aa 100644 --- a/lib/internal/modules/esm/initialize_import_meta.js +++ b/lib/internal/modules/esm/initialize_import_meta.js @@ -3,12 +3,9 @@ const { getOptionValue } = require('internal/options'); const experimentalImportMetaResolve = getOptionValue('--experimental-import-meta-resolve'); -const { fetchModule } = require('internal/modules/esm/fetch_module'); -const { URL } = require('internal/url'); const { PromisePrototypeThen, PromiseReject, - StringPrototypeStartsWith, } = primordials; const asyncESM = require('internal/process/esm_loader'); @@ -24,6 +21,10 @@ function createImportMetaResolve(defaultParentUrl) { }; } +/** + * @param {object} meta + * @param {{url: string}} context + */ function initializeImportMeta(meta, context) { let url = context.url; @@ -32,14 +33,7 @@ function initializeImportMeta(meta, context) { meta.resolve = createImportMetaResolve(url); } - if ( - StringPrototypeStartsWith(url, 'http:') || - StringPrototypeStartsWith(url, 'https:') - ) { - // The request & response have already settled, so they are in fetchModule's - // cache, in which case, fetchModule returns immediately and synchronously - url = fetchModule(new URL(url), context).resolvedHREF; - } + url = asyncESM.esmLoader.getBaseURL(url); meta.url = url; } diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 61e609d9ad6c62..16b832527f7c64 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -17,11 +17,13 @@ const { RegExpPrototypeExec, SafeArrayIterator, SafeWeakMap, + StringPrototypeStartsWith, globalThis, } = primordials; const { MessageChannel } = require('internal/worker/io'); const { + ERR_INTERNAL_ASSERTION, ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, ERR_INVALID_RETURN_PROPERTY_VALUE, @@ -47,6 +49,9 @@ const { defaultLoad } = require('internal/modules/esm/load'); const { translators } = require( 'internal/modules/esm/translators'); const { getOptionValue } = require('internal/options'); +const { + fetchModule, +} = require('internal/modules/esm/fetch_module'); /** * An ESMLoader instance is used as the main entry point for loading ES modules. @@ -209,7 +214,9 @@ class ESMLoader { const module = new ModuleWrap(url, undefined, source, 0, 0); callbackMap.set(module, { importModuleDynamically: (specifier, { url }, importAssertions) => { - return this.import(specifier, url, importAssertions); + return this.import(specifier, + this.getBaseURL(url), + importAssertions); } }); @@ -225,6 +232,43 @@ class ESMLoader { }; } + /** + * Returns the url to use for the resolution of a given cache key url + * These are not guaranteed to be the same. + * + * In WHATWG HTTP spec for ESM the cache key is the non-I/O bound + * synchronous resolution using only string operations + * ~= resolveImportMap(new URL(specifier, importerHREF)) + * + * The url used for subsequent resolution is the response URL after + * all redirects have been resolved. + * + * https://example.com/foo redirecting to https://example.com/bar + * would have a cache key of https://example.com/foo and baseURL + * of https://example.com/bar + * + * MUST BE SYNCHRONOUS for import.meta initialization + * MUST BE CALLED AFTER receiving the url body due to I/O + * @param {string} url + * @returns {string} + */ + getBaseURL(url) { + if ( + StringPrototypeStartsWith(url, 'http:') || + StringPrototypeStartsWith(url, 'https:') + ) { + // The request & response have already settled, so they are in + // fetchModule's cache, in which case, fetchModule returns + // immediately and synchronously + url = fetchModule(new URL(url), { parentURL: url }).resolvedHREF; + // This should only occur if the module hasn't been fetched yet + if (typeof url !== 'string') { + throw new ERR_INTERNAL_ASSERTION(`Base url for module ${url} not loaded.`); + } + } + return url; + } + /** * Get a (possibly still pending) module job from the cache, * or create one and return its Promise. diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index fd1c6166330e76..e012eebc4ac971 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -76,7 +76,8 @@ class ModuleJob { // these `link` callbacks depending on each other. const dependencyJobs = []; const promises = this.module.link(async (specifier, assertions) => { - const jobPromise = this.loader.getModuleJob(specifier, url, assertions); + const baseURL = this.loader.getBaseURL(url); + const jobPromise = this.loader.getModuleJob(specifier, baseURL, assertions); ArrayPrototypePush(dependencyJobs, jobPromise); const job = await jobPromise; return job.modulePromise; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index da95703faad9fd..d7f4c7edec63d3 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -103,7 +103,9 @@ function errPath(url) { } async function importModuleDynamically(specifier, { url }, assertions) { - return asyncESM.esmLoader.import(specifier, url, assertions); + return asyncESM.esmLoader.import(specifier, + asyncESM.esmLoader.getBaseURL(url), + assertions); } // Strategy for loading a standard JavaScript module. diff --git a/test/es-module/test-http-imports.mjs b/test/es-module/test-http-imports.mjs index 139c72a4e26c65..a1a208689f054f 100644 --- a/test/es-module/test-http-imports.mjs +++ b/test/es-module/test-http-imports.mjs @@ -111,6 +111,21 @@ for (const { protocol, createServer } of [ assert.strict.notEqual(redirectedNS.default, ns.default); assert.strict.equal(redirectedNS.url, url.href); + // Redirects have the same import.meta.url but different cache + // entry on Web + const relativeAfterRedirect = new URL(url.href + 'foo/index.js'); + const redirected = new URL(url.href + 'bar/index.js'); + redirected.searchParams.set('body', 'export let relativeDepURL = (await import("./baz.js")).url'); + relativeAfterRedirect.searchParams.set('redirect', JSON.stringify({ + status: 302, + location: redirected.href + })); + const relativeAfterRedirectedNS = await import(relativeAfterRedirect.href); + assert.strict.equal( + relativeAfterRedirectedNS.relativeDepURL, + url.href + 'bar/baz.js' + ); + const crossProtocolRedirect = new URL(url.href); crossProtocolRedirect.searchParams.set('redirect', JSON.stringify({ status: 302,