8000 esm: fix cache collision on JSON files using file: URL · nodejs/node@7f0e36a · GitHub
[go: up one dir, main page]
Skip to content

Commit 7f0e36a

Browse files
aduh95aduh95
authored
esm: fix cache collision on JSON files using file: URL
PR-URL: #49887 Fixes: #49724 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent 8530180 commit 7f0e36a

File tree

3 files changed

+98
-4
lines changed

3 files changed

+98
-4
lines changed

‎lib/internal/modules/esm/translators.js‎

Lines changed: 7 additions & 3 deletions
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,
@@ -443,9 +444,12 @@ translators.set('json', function jsonStrategy(url, source) {
443444
debug(`Loading JSONModule ${url}`);
444445
const pathname = StringPrototypeStartsWith(url, 'file:') ?
445446
fileURLToPath(url) : null;
447+
const shouldCheckAndPopulateCJSModuleCache =
448+
// We want to involve the CJS loader cache only for `file:` URL with no search query and no hash.
449+
pathname && !StringPrototypeIncludes(url, '?') && !StringPrototypeIncludes(url, '#');
446450
let modulePath;
447451
let module;
448-
if (pathname) {
452+
if (shouldCheckAndPopulateCJSModuleCache) {
449453
modulePath = isWindows ?
450454
StringPrototypeReplaceAll(pathname, '/', '\\') : pathname;
451455
module = CJSModule._cache[modulePath];
@@ -457,7 +461,7 @@ translators.set('json', function jsonStrategy(url, source) {
457461
}
458462
}
459463
source = stringify(source);
460-
if (pathname) {
464+
if (shouldCheckAndPopulateCJSModuleCache) {
461465
// A require call could have been called on the same file during loading and
462466
// that resolves synchronously. To make sure we always return the identical
463467
// export, we have to check again if the module already exists or not.
@@ -484,7 +488,7 @@ translators.set('json', function jsonStrategy(url, source) {
484488
err.message = errPath(url) + ': ' + err.message;
485489
throw err;
486490
}
487-
if (pathname) {
491+
if (shouldCheckAndPopulateCJSModuleCache) {
488492
CJSModule._cache[modulePath] = module;
489493
}
490494
cjsCache.set(url, module);

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

Lines changed: 61 additions & 1 deletion
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
});

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

Lines changed: 30 additions & 0 deletions
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)
0