module: add application/json in accept header when fetching json module

PR-URL: #50119 Refs: #50116 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
nodejs · Nov 11, 2023 · 66d793d · 66d793d
1 parent de85aca
commit 66d793d
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 6 deletions.
diff --git a/lib/internal/modules/esm/fetch_module.js b/lib/internal/modules/esm/fetch_module.js
@@ -134,19 +134,32 @@ function isRedirect(statusCode) {
   }
 }
 
+/**
+ * @typedef AcceptMimes possible values of Accept header when fetching a module
+ * @property {Promise<string> | string} default default Accept header value.
+ * @property {Record<string, string>} json Accept header value when fetching module with importAttributes json.
+ * @type {AcceptMimes}
+ */
+const acceptMimes = {
+  __proto_: null,
+  default: '*/*',
+  json: 'application/json,*/*;charset=utf-8;q=0.5',
+};
+
 /**
  * @param {URL} parsed
  * @returns {Promise<CacheEntry> | 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 accept = acceptMimes[context.importAttributes?.type] ?? acceptMimes.default;
     const req = handler(parsed, {
-      headers: { Accept: '*/*' },
+      headers: { Accept: accept },
     });
     // Note that `once` is used here to handle `error` and that it hits the
     // `finally` on network error/timeout.
@@ -162,7 +175,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 +275,8 @@ async function isLocalAddress(hostname) {
  * @param {ESModuleContext} context
  * @returns {ReturnType<typeof fetchWithRedirects>}
  */
-function fetchModule(parsed, { parentURL }) {
+function fetchModule(parsed, context) {
+  const { parentURL } = context;
   const { href } = parsed;
   const existing = cacheForGET.get(href);
   if (existing) {
@@ -277,10 +291,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
@@ -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,*/*;charset=utf-8;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, { with: { type: 'json' } });
+    assert.deepStrictEqual(Object.keys(json), ['default']);
+    assert.strictEqual(json.default.x, 1);
+
     server.close();
   }
 }