esm: fix cache collision on JSON files using file: URL

nodejs · nodejs-github-bot · Sep 29, 2023 · Sep 26, 2023 · Sep 26, 2023 · Sep 27, 2023
commit 8b2861a8282cb048829b7b86ddb36d136f83e56b
diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
@@ -11,6 +11,7 @@ const {
   SafeArrayIterator,
   SafeMap,
   SafeSet,
+  StringPrototypeIncludes,
   StringPrototypeReplaceAll,
   StringPrototypeSlice,
   StringPrototypeStartsWith,
@@ -443,9 +444,11 @@ translators.set('json', function jsonStrategy(url, source) {
   debug(`Loading JSONModule ${url}`);
   const pathname = StringPrototypeStartsWith(url, 'file:') ?
     fileURLToPath(url) : null;
+  const shouldCheckAndPopulateCJSModuleCache =
+    pathname && !StringPrototypeIncludes(url, '?') && !StringPrototypeIncludes(url, '#');
   let modulePath;
   let module;
-  if (pathname) {
+  if (shouldCheckAndPopulateCJSModuleCache) {
     modulePath = isWindows ?
       StringPrototypeReplaceAll(pathname, '/', '\\') : pathname;
     module = CJSModule._cache[modulePath];
@@ -457,7 +460,7 @@ translators.set('json', function jsonStrategy(url, source) {
     }
   }
   source = stringify(source);
-  if (pathname) {
+  if (shouldCheckAndPopulateCJSModuleCache) {
     // A require call could have been called on the same file during loading and
     // that resolves synchronously. To make sure we always return the identical
     // export, we have to check again if the module already exists or not.
@@ -484,7 +487,7 @@ translators.set('json', function jsonStrategy(url, source) {
     err.message = errPath(url) + ': ' + err.message;
     throw err;
   }
-  if (pathname) {
+  if (shouldCheckAndPopulateCJSModuleCache) {
     CJSModule._cache[modulePath] = module;
   }
   cjsCache.set(url, module);

diff --git a/test/es-module/test-esm-json.mjs b/test/es-module/test-esm-json.mjs
@@ -2,7 +2,10 @@ import { spawnPromisified } from '../common/index.mjs';
 import * as fixtures from '../common/fixtures.mjs';
 import assert from 'node:assert';
 import { execPath } from 'node:process';
-import { describe, it } from 'node:test';
+import { describe, it, test } from 'node:test';
+
+import { mkdir, rm, writeFile } from 'node:fs/promises';
+import * as tmpdir from '../common/tmpdir.js';
 
 import secret from '../fixtures/experimental.json' assert { type: 'json' };
 
@@ -21,4 +24,40 @@ describe('ESM: importing JSON', () => {
     assert.strictEqual(code, 0);
     assert.strictEqual(signal, null);
   });
+
+  test('should load different modules when the URL is different', async (t) => {
+    const root = tmpdir.fileurl("https://github.com/nodejs/node/pull/49887/commits/%60./test-esm-json-$%7BMath.random(")}/`);
+    try {
+      await mkdir(root, { recursive: true });
+
+      await t.test('json', async () => {
+        const url = new url("https://github.com/nodejs/node/pull/49887/commits/'./foo.json',%20root");
+        await writeFile(url, JSON.stringify({ firstJson: true }));
+        const firstJson = await import(`${url}?a=1`, {
+          assert: { type: 'json' },
+          });
+        await writeFile(url, JSON.stringify({ firstJson: false }));
+        const secondJson = await import(`${url}?a=2`, {
+          assert: { type: 'json' },
+          });
+
+        assert.notDeepStrictEqual(firstJson, secondJson);
+      });
+
+      await t.test('js', async () => {
+        const url = new url("https://github.com/nodejs/node/pull/49887/commits/'./foo.mjs',%20root");
+        await writeFile(url, `
+      export const firstJS = true
+      `);
+        const firstJS = await import(`${url}?a=1`);
+        await writeFile(url, `
+      export const firstJS = false
+      `);
+        const secondJS = await import(`${url}?a=2`);
+        assert.notDeepStrictEqual(firstJS, secondJS);
+      });
+    } finally {
+      await rm(root, { force: true, recursive: true });
+    }
+  });
 });
diff --git a/test/es-module/test-esm-virtual-json.mjs b/test/es-module/test-esm-virtual-json.mjs
@@ -0,0 +1,30 @@
+import '../common/index.mjs';
+import * as fixtures from '../common/fixtures.mjs';
+import { register } from 'node:module';
+import assert from 'node:assert';
+
+async function resolve(referrer, context, next) {
+  const result = await next(referrer, context);
+  const url = new url("https://github.com/nodejs/node/pull/49887/commits/result.url");
+  url.searchParams.set('randomSeed', Math.random());
+  result.url = url.href;
+  return result;
+}
+
+function load(url, context, next) {
+  if (context.importAssertions.type === 'json') {
+    return {
+      shortCircuit: true,
+      format: 'json',
+      source: JSON.stringify({ data: Math.random() }),
+    };
+  }
+  return next(url, context);
+}
+
+register(`data:text/javascript,export ${encodeURIComponent(resolve)};export ${encodeURIComponent(load)}`);
+
+assert.notDeepStrictEqual(
+  await import(fixtures.fileurl("https://github.com/nodejs/node/pull/49887/commits/'empty.json'"), { assert: { type: 'json' } }),
+  await import(fixtures.fileurl("https://github.com/nodejs/node/pull/49887/commits/'empty.json'"), { assert: { type: 'json' } }),
+);