Skip to content

Commit a918cd8

Browse files
committed
vfs: avoid recursive readdir symlink cycles
Track the active MemoryProvider recursive readdir traversal path so circular symlinks to directories stop recursing while still listing the symlink entries. Add sync, promise, and withFileTypes coverage. Fixes: #64148 Signed-off-by: Matteo Collina <hello@matteocollina.com>
1 parent 54296cb commit a918cd8

2 files changed

Lines changed: 96 additions & 31 deletions

File tree

lib/internal/vfs/providers/memory.js

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const {
55
ArrayPrototypePush,
66
DateNow,
77
SafeMap,
8+
SafeSet,
89
StringPrototypeReplaceAll,
910
Symbol,
1011
} = primordials;
@@ -611,44 +612,55 @@ class MemoryProvider extends VirtualProvider {
611612
*/
612613
#readdirRecursive(dirEntry, dirPath, withFileTypes) {
613614
const results = [];
615+
const active = new SafeSet();
614616

615617
const walk = (entry, currentPath, relativePath) => {
616-
this.#ensurePopulated(entry, currentPath);
617-
618-
for (const { 0: name, 1: childEntry } of entry.children) {
619-
const childRelative = relativePath ?
620-
relativePath + '/' + name : name;
618+
if (active.has(entry)) {
619+
return;
620+
}
621621

622-
if (withFileTypes) {
623-
let type;
624-
if (childEntry.isSymbolicLink()) {
625-
type = UV_DIRENT_LINK;
626-
} else if (childEntry.isDirectory()) {
627-
type = UV_DIRENT_DIR;
622+
active.add(entry);
623+
try {
624+
this.#ensurePopulated(entry, currentPath);
625+
626+
for (const { 0: name, 1: childEntry } of entry.children) {
627+
const childRelative = relativePath ?
628+
relativePath + '/' + name : name;
629+
630+
if (withFileTypes) {
631+
let type;
632+
if (childEntry.isSymbolicLink()) {
633+
type = UV_DIRENT_LINK;
634+
} else if (childEntry.isDirectory()) {
635+
type = UV_DIRENT_DIR;
636+
} else {
637+
type = UV_DIRENT_FILE;
638+
}
639+
ArrayPrototypePush(results,
640+
new Dirent(childRelative, type, dirPath));
628641
} else {
629-
type = UV_DIRENT_FILE;
642+
ArrayPrototypePush(results, childRelative);
630643
}
631-
ArrayPrototypePush(results,
632-
new Dirent(childRelative, type, dirPath));
633-
} else {
634-
ArrayPrototypePush(results, childRelative);
635-
}
636644

637-
// Follow symlinks to directories for recursive traversal
638-
let resolvedChild = childEntry;
639-
if (childEntry.isSymbolicLink()) {
640-
const targetPath = this.#resolveSymlinkTarget(
641-
pathPosix.join(currentPath, name), childEntry.target,
642-
);
643-
const result = this.#lookupEntry(targetPath, true, 0);
644-
if (result.entry) {
645-
resolvedChild = result.entry;
645+
// Follow symlinks to directories for recursive traversal.
646+
// Track the active traversal path to avoid symlink cycles.
647+
let resolvedChild = childEntry;
648+
if (childEntry.isSymbolicLink()) {
649+
const targetPath = this.#resolveSymlinkTarget(
650+
pathPosix.join(currentPath, name), childEntry.target,
651+
);
652+
const result = this.#lookupEntry(targetPath, true, 0);
653+
if (result.entry) {
654+
resolvedChild = result.entry;
655+
}
656+
}
657+
if (resolvedChild.isDirectory()) {
658+
const childPath = pathPosix.join(currentPath, name);
659+
walk(resolvedChild, childPath, childRelative);
646660
}
647661
}
648-
if (resolvedChild.isDirectory()) {
649-
const childPath = pathPosix.join(currentPath, name);
650-
walk(resolvedChild, childPath, childRelative);
651-
}
662+
} finally {
663+
active.delete(entry);
652664
}
653665
};
654666

test/parallel/test-vfs-readdir-symlink-recursive.js

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
// Recursive readdir must follow symlinks to directories.
55

6-
require('../common');
6+
const common = require('../common');
77
const assert = require('assert');
88
const vfs = require('node:vfs');
99

@@ -21,6 +21,59 @@ assert.ok(
2121
`Expected 'symdir/nested.txt' in entries: ${entries}`,
2222
);
2323

24+
// Recursive readdir avoids following symlink cycles indefinitely.
25+
{
26+
const v = vfs.create();
27+
v.mkdirSync('/dir');
28+
v.writeFileSync('/dir/nested.txt', 'nested');
29+
v.symlinkSync('/dir', '/dir/loop');
30+
31+
assert.deepStrictEqual(v.readdirSync('/', { recursive: true }).sort(), [
32+
'dir',
33+
'dir/loop',
34+
'dir/nested.txt',
35+
]);
36+
37+
const dirents = v.readdirSync('/', { recursive: true, withFileTypes: true });
38+
assert.strictEqual(dirents.length, 3);
39+
assert.ok(dirents.some((dirent) =>
40+
dirent.name === 'loop' &&
41+
dirent.parentPath === '/dir' &&
42+
dirent.isSymbolicLink()));
43+
}
44+
45+
// Recursive readdir avoids cycles through multiple symlinks.
46+
{
47+
const v = vfs.create();
48+
v.mkdirSync('/a');
49+
v.mkdirSync('/b');
50+
v.symlinkSync('/b', '/a/link_to_b');
51+
v.symlinkSync('/a', '/b/link_to_a');
52+
53+
assert.deepStrictEqual(v.readdirSync('/', { recursive: true }).sort(), [
54+
'a',
55+
'a/link_to_b',
56+
'a/link_to_b/link_to_a',
57+
'b',
58+
'b/link_to_a',
59+
'b/link_to_a/link_to_b',
60+
]);
61+
}
62+
63+
(async () => {
64+
const v = vfs.create();
65+
v.mkdirSync('/dir');
66+
v.writeFileSync('/dir/nested.txt', 'nested');
67+
v.symlinkSync('/dir', '/dir/loop');
68+
69+
const entries = await v.promises.readdir('/', { recursive: true });
70+
assert.deepStrictEqual(entries.sort(), [
71+
'dir',
72+
'dir/loop',
73+
'dir/nested.txt',
74+
]);
75+
})().then(common.mustCall());
76+
2477
// Recursive readdir with withFileTypes:true returns Dirent objects whose
2578
// parentPath reflects the actual location of the entry (not the entry's
2679
// stringified relative path).

0 commit comments

Comments
 (0)