Skip to content

Commit c12685f

Browse files
aduh95targos
authored andcommitted
esm: fix cache collision on JSON files using file: URL
PR-URL: #49887 Backport-PR-URL: #50669 Fixes: #49724 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
1 parent ed8dd33 commit c12685f

File tree

3 files changed

+98
-4
lines changed

3 files changed

+98
-4
lines changed

lib/internal/modules/esm/translators.js

+7-3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
SafeArrayIterator,
1212
SafeMap,
1313
SafeSet,
14+
StringPrototypeIncludes,
1415
StringPrototypeReplaceAll,
1516
StringPrototypeSlice,
1617
StringPrototypeStartsWith,
@@ -265,9 +266,12 @@ translators.set('json', async function jsonStrategy(url, source) {
265266
debug(`Loading JSONModule ${url}`);
266267
const pathname = StringPrototypeStartsWith(url, 'file:') ?
267268
fileURLToPath(url) : null;
269+
const shouldCheckAndPopulateCJSModuleCache =
270+
// We want to involve the CJS loader cache only for `file:` URL with no search query and no hash.
271+
pathname && !StringPrototypeIncludes(url, '?') && !StringPrototypeIncludes(url, '#');
268272
let modulePath;
269273
let module;
270-
if (pathname) {
274+
if (shouldCheckAndPopulateCJSModuleCache) {
271275
modulePath = isWindows ?
272276
StringPrototypeReplaceAll(pathname, '/', '\\') : pathname;
273277
module = CJSModule._cache[modulePath];
@@ -279,7 +283,7 @@ translators.set('json', async function jsonStrategy(url, source) {
279283
}
280284
}
281285
source = stringify(source);
282-
if (pathname) {
286+
if (shouldCheckAndPopulateCJSModuleCache) {
283287
// A require call could have been called on the same file during loading and
284288
// that resolves synchronously. To make sure we always return the identical
285289
// export, we have to check again if the module already exists or not.
@@ -305,7 +309,7 @@ translators.set('json', async function jsonStrategy(url, source) {
305309
err.message = errPath(url) + ': ' + err.message;
306310
throw err;
307311
}
308-
if (pathname) {
312+
if (shouldCheckAndPopulateCJSModuleCache) {
309313
CJSModule._cache[modulePath] = module;
310314
}
311315
return new ModuleWrap(url, undefined, ['default'], function() {

test/es-module/test-esm-json.mjs

+61-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ import { spawnPromisified } from '../common/index.mjs';
22
import * as fixtures from '../common/fixtures.mjs';
33
import assert from 'node:assert';
44
import { execPath } from 'node:process';
5-
import { describe, it } from 'node:test';
5+
import { describe, it, test } from 'node:test';
6+
7+
import { mkdir, rm, writeFile } from 'node:fs/promises';
8+
import * as tmpdir from '../common/tmpdir.js';
69

710
import secret from '../fixtures/experimental.json' assert { type: 'json' };
811

@@ -21,4 +24,61 @@ describe('ESM: importing JSON', () => {
2124
assert.strictEqual(code, 0);
2225
assert.strictEqual(signal, null);
2326
});
27+
28+
test('should load different modules when the URL is different', async (t) => {
29+
const root = tmpdir.fileURL(`./test-esm-json-${Math.random()}/`);
30+
try {
31+
await mkdir(root, { recursive: true });
32+
33+
await t.test('json', async () => {
34+
let i = 0;
35+
const url = new URL('./foo.json', root);
36+
await writeFile(url, JSON.stringify({ id: i++ }));
37+
const absoluteURL = await import(`${url}`, {
38+
assert: { type: 'json' },
39+
});
40+
await writeFile(url, JSON.stringify({ id: i++ }));
41+
const queryString = await import(`${url}?a=2`, {
42+
assert: { type: 'json' },
43+
});
44+
await writeFile(url, JSON.stringify({ id: i++ }));
45+
const hash = await import(`${url}#a=2`, {
46+
assert: { type: 'json' },
47+
});
48+
await writeFile(url, JSON.stringify({ id: i++ }));
49+
const queryStringAndHash = await import(`${url}?a=2#a=2`, {
50+
assert: { type: 'json' },
51+
});
52+
53+
assert.notDeepStrictEqual(absoluteURL, queryString);
54+
assert.notDeepStrictEqual(absoluteURL, hash);
55+
assert.notDeepStrictEqual(queryString, hash);
56+
assert.notDeepStrictEqual(absoluteURL, queryStringAndHash);
57+
assert.notDeepStrictEqual(queryString, queryStringAndHash);
58+
assert.notDeepStrictEqual(hash, queryStringAndHash);
59+
});
60+
61+
await t.test('js', async () => {
62+
let i = 0;
63+
const url = new URL('./foo.mjs', root);
64+
await writeFile(url, `export default ${JSON.stringify({ id: i++ })}\n`);
65+
const absoluteURL = await import(`${url}`);
66+
await writeFile(url, `export default ${JSON.stringify({ id: i++ })}\n`);
67+
const queryString = await import(`${url}?a=1`);
68+
await writeFile(url, `export default ${JSON.stringify({ id: i++ })}\n`);
69+
const hash = await import(`${url}#a=1`);
70+
await writeFile(url, `export default ${JSON.stringify({ id: i++ })}\n`);
71+
const queryStringAndHash = await import(`${url}?a=1#a=1`);
72+
73+
assert.notDeepStrictEqual(absoluteURL, queryString);
74+
assert.notDeepStrictEqual(absoluteURL, hash);
75+
assert.notDeepStrictEqual(queryString, hash);
76+
assert.notDeepStrictEqual(absoluteURL, queryStringAndHash);
77+
assert.notDeepStrictEqual(queryString, queryStringAndHash);
78+
assert.notDeepStrictEqual(hash, queryStringAndHash);
79+
});
80+
} finally {
81+
await rm(root, { force: true, recursive: true });
82+
}
83+
});
2484
});
+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import '../common/index.mjs';
2+
import * as fixtures from '../common/fixtures.mjs';
3+
import { register } from 'node:module';
4+
import assert from 'node:assert';
5+
6+
async function resolve(referrer, context, next) {
7+
const result = await next(referrer, context);
8+
const url = new URL(result.url);
9+
url.searchParams.set('randomSeed', Math.random());
10+
result.url = url.href;
11+
return result;
12+
}
13+
14+
function load(url, context, next) {
15+
if (context.importAssertions.type === 'json') {
16+
return {
17+
shortCircuit: true,
18+
format: 'json',
19+
source: JSON.stringify({ data: Math.random() }),
20+
};
21+
}
22+
return next(url, context);
23+
}
24+
25+
register(`data:text/javascript,export ${encodeURIComponent(resolve)};export ${encodeURIComponent(load)}`);
26+
27+
assert.notDeepStrictEqual(
28+
await import(fixtures.fileURL('empty.json'), { assert: { type: 'json' } }),
29+
await import(fixtures.fileURL('empty.json'), { assert: { type: 'json' } }),
30+
);

0 commit comments

Comments
 (0)