From d47163347ec6605895f65a456b60c558e1cd5852 Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Tue, 10 Oct 2023 11:47:36 +0100 Subject: [PATCH 1/4] module: add application/json in accept header when fetching json module --- lib/internal/modules/esm/fetch_module.js | 14 ++++++++------ test/es-module/test-http-imports.mjs | 12 ++++++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/lib/internal/modules/esm/fetch_module.js b/lib/internal/modules/esm/fetch_module.js index 21b7456899604f..c08645554b9e9d 100644 --- a/lib/internal/modules/esm/fetch_module.js +++ b/lib/internal/modules/esm/fetch_module.js @@ -138,15 +138,16 @@ function isRedirect(statusCode) { * @param {URL} parsed * @returns {Promise | CacheEntry} */ -function fetchWithRedirects(parsed) { +function fetchWithRedirects(parsed, context) { const existing = cacheForGET.get(parsed.href); if (existing) { return existing; } const handler = parsed.protocol === 'http:' ? HTTPGet : HTTPSGet; const result = (async () => { + const type = context.importAssertions?.type; const req = handler(parsed, { - headers: { Accept: '*/*' }, + headers: { Accept: type === 'json' ? 'application/json,*/*;q=0.5' : '*/*;' }, }); // Note that `once` is used here to handle `error` and that it hits the // `finally` on network error/timeout. @@ -162,7 +163,7 @@ function fetchWithRedirects(parsed) { 'cannot redirect to non-network location', ); } - const entry = await fetchWithRedirects(location); + const entry = await fetchWithRedirects(location, context); cacheForGET.set(parsed.href, entry); return entry; } @@ -262,7 +263,8 @@ async function isLocalAddress(hostname) { * @param {ESModuleContext} context * @returns {ReturnType} */ -function fetchModule(parsed, { parentURL }) { +function fetchModule(parsed, context) { + const { parentURL } = context; const { href } = parsed; const existing = cacheForGET.get(href); if (existing) { @@ -277,10 +279,10 @@ function fetchModule(parsed, { parentURL }) { 'http can only be used to load local resources (use https instead).', ); } - return fetchWithRedirects(parsed); + return fetchWithRedirects(parsed, context); }); } - return fetchWithRedirects(parsed); + return fetchWithRedirects(parsed, context); } module.exports = { diff --git a/test/es-module/test-http-imports.mjs b/test/es-module/test-http-imports.mjs index 235d142d3555e3..8a19c00b0b413a 100644 --- a/test/es-module/test-http-imports.mjs +++ b/test/es-module/test-http-imports.mjs @@ -68,6 +68,11 @@ for (const { protocol, createServer } of [ const server = createServer(function(_req, res) { const url = new URL(_req.url, host); const redirect = url.searchParams.get('redirect'); + + if (url.pathname === 'json') { + common.mustCall(() => assert.strictEqual(_req.header.content, 'application/json,*/*;q=0.5')); + } + if (url.pathname === '/not-found') { res.writeHead(404); res.end(); @@ -204,6 +209,13 @@ for (const { protocol, createServer } of [ { code: 'ERR_MODULE_NOT_FOUND' }, ); + const jsonUrl = new URL(url.href + 'json'); + jsonUrl.searchParams.set('mime', 'application/json'); + jsonUrl.searchParams.set('body', '{"x": 1}'); + const json = await import(jsonUrl.href, { assert: { type: 'json' } }); + assert.deepStrictEqual(Object.keys(json), ['default']); + assert.strictEqual(json.default.x, 1); + server.close(); } } From 27c59a3d0378e686eb4359b456593f3e1a8b9c75 Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Sat, 14 Oct 2023 09:08:44 +0100 Subject: [PATCH 2/4] fix: renamed to attributes --- lib/internal/modules/esm/fetch_module.js | 2 +- test/es-module/test-http-imports.mjs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/esm/fetch_module.js b/lib/internal/modules/esm/fetch_module.js index c08645554b9e9d..4f3c1f028193eb 100644 --- a/lib/internal/modules/esm/fetch_module.js +++ b/lib/internal/modules/esm/fetch_module.js @@ -145,7 +145,7 @@ function fetchWithRedirects(parsed, context) { } const handler = parsed.protocol === 'http:' ? HTTPGet : HTTPSGet; const result = (async () => { - const type = context.importAssertions?.type; + const type = context.importAttributes?.type; const req = handler(parsed, { headers: { Accept: type === 'json' ? 'application/json,*/*;q=0.5' : '*/*;' }, }); diff --git a/test/es-module/test-http-imports.mjs b/test/es-module/test-http-imports.mjs index 8a19c00b0b413a..5bfb426fffc36d 100644 --- a/test/es-module/test-http-imports.mjs +++ b/test/es-module/test-http-imports.mjs @@ -212,7 +212,7 @@ for (const { protocol, createServer } of [ const jsonUrl = new URL(url.href + 'json'); jsonUrl.searchParams.set('mime', 'application/json'); jsonUrl.searchParams.set('body', '{"x": 1}'); - const json = await import(jsonUrl.href, { assert: { type: 'json' } }); + const json = await import(jsonUrl.href, { with: { type: 'json' } }); assert.deepStrictEqual(Object.keys(json), ['default']); assert.strictEqual(json.default.x, 1); From 1dcd38968922fb764a5170c91340dad3bea5a3ee Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Wed, 18 Oct 2023 12:35:50 +0200 Subject: [PATCH 3/4] fix: make it acceptedMimes generic --- lib/internal/modules/esm/fetch_module.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/esm/fetch_module.js b/lib/internal/modules/esm/fetch_module.js index 4f3c1f028193eb..05d33cf883c47f 100644 --- a/lib/internal/modules/esm/fetch_module.js +++ b/lib/internal/modules/esm/fetch_module.js @@ -134,6 +134,18 @@ function isRedirect(statusCode) { } } +/** + * @typedef AcceptMimes possible values of Accept header when fetching a module + * @property {Promise | string} default default Accept header value. + * @property {Record} json Accept header value when fetching module with importAttributes json. + * @type {AcceptMimes} + */ +const acceptMimes = { + __proto_: null, + default: '*/*', + json: 'application/json,*/*;q=0.5', +}; + /** * @param {URL} parsed * @returns {Promise | CacheEntry} @@ -145,9 +157,9 @@ function fetchWithRedirects(parsed, context) { } const handler = parsed.protocol === 'http:' ? HTTPGet : HTTPSGet; const result = (async () => { - const type = context.importAttributes?.type; + const accept = acceptMimes[context.importAttributes?.type] ?? acceptMimes.default; const req = handler(parsed, { - headers: { Accept: type === 'json' ? 'application/json,*/*;q=0.5' : '*/*;' }, + headers: { Accept: accept }, }); // Note that `once` is used here to handle `error` and that it hits the // `finally` on network error/timeout. From d6875ef1ef78b6059cd43c477fcf3135618d9b3b Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Mon, 30 Oct 2023 10:28:36 +0200 Subject: [PATCH 4/4] feat: add charset to header --- lib/internal/modules/esm/fetch_module.js | 2 +- test/es-module/test-http-imports.mjs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/esm/fetch_module.js b/lib/internal/modules/esm/fetch_module.js index 05d33cf883c47f..b3491d97cb99c7 100644 --- a/lib/internal/modules/esm/fetch_module.js +++ b/lib/internal/modules/esm/fetch_module.js @@ -143,7 +143,7 @@ function isRedirect(statusCode) { const acceptMimes = { __proto_: null, default: '*/*', - json: 'application/json,*/*;q=0.5', + json: 'application/json,*/*;charset=utf-8;q=0.5', }; /** diff --git a/test/es-module/test-http-imports.mjs b/test/es-module/test-http-imports.mjs index 5bfb426fffc36d..e1cb1f5ced59c7 100644 --- a/test/es-module/test-http-imports.mjs +++ b/test/es-module/test-http-imports.mjs @@ -70,7 +70,7 @@ for (const { protocol, createServer } of [ const redirect = url.searchParams.get('redirect'); if (url.pathname === 'json') { - common.mustCall(() => assert.strictEqual(_req.header.content, 'application/json,*/*;q=0.5')); + common.mustCall(() => assert.strictEqual(_req.header.content, 'application/json,*/*;charset=utf-8;q=0.5')); } if (url.pathname === '/not-found') {