Skip to content

Commit e9333ce

Browse files
vfs: fix rename over non-empty directory
The memory provider only rejected renames whose destination had a different type than the source, so renaming a directory onto another directory silently dropped the destination and everything under it. An existing destination directory has to be empty; rename(2) reports ENOTEMPTY otherwise, and RealFSProvider already does so because it delegates to fs.renameSync(). Also, decrement nlink on a file that is replaced by a rename, and make a rename whose two names resolve to the same entry a no-op, which covers renaming one hard link onto another. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>
1 parent 29c517f commit e9333ce

2 files changed

Lines changed: 76 additions & 0 deletions

File tree

‎lib/internal/vfs/providers/memory.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,10 @@ class MemoryProvider extends VirtualProvider {
840840

841841
// Check if destination exists
842842
const existingDest = newParent.children.get(newName);
843+
if (existingDest === entry) {
844+
// Both names resolve to the same entry: rename does nothing
845+
return;
846+
}
843847
if (existingDest) {
844848
// Cannot overwrite a directory with a non-directory
845849
if (existingDest.isDirectory() && !entry.isDirectory()) {
@@ -849,6 +853,14 @@ class MemoryProvider extends VirtualProvider {
849853
if (!existingDest.isDirectory() && entry.isDirectory()) {
850854
throw createENOTDIR('rename', newPath);
851855
}
856+
if (existingDest.isDirectory()) {
857+
// Cannot overwrite a non-empty directory
858+
if (existingDest.children.size > 0) {
859+
throw createENOTEMPTY('rename', newPath);
860+
}
861+
} else {
862+
existingDest.nlink--;
863+
}
852864
}
853865

854866
// Remove from old location (after destination validation)

‎test/parallel/test-vfs-rename.js‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,67 @@ const vfs = require('node:vfs');
5858
assert.strictEqual(myVfs.existsSync('/a/b/c'), false);
5959
assert.strictEqual(myVfs.readFileSync('/a/file.txt', 'utf8'), 'data');
6060
}
61+
62+
// Renaming a directory onto a non-empty directory throws ENOTEMPTY
63+
{
64+
const myVfs = vfs.create();
65+
myVfs.mkdirSync('/src');
66+
myVfs.mkdirSync('/dst');
67+
myVfs.writeFileSync('/dst/keep.txt', 'keep');
68+
69+
assert.throws(() => myVfs.renameSync('/src', '/dst'), { code: 'ENOTEMPTY' });
70+
assert.strictEqual(myVfs.readFileSync('/dst/keep.txt', 'utf8'), 'keep');
71+
assert.strictEqual(myVfs.existsSync('/src'), true);
72+
}
73+
74+
// Renaming a directory onto an empty directory succeeds
75+
{
76+
const myVfs = vfs.create();
77+
myVfs.mkdirSync('/src');
78+
myVfs.writeFileSync('/src/a.txt', 'a');
79+
myVfs.mkdirSync('/dst');
80+
81+
myVfs.renameSync('/src', '/dst');
82+
assert.strictEqual(myVfs.existsSync('/src'), false);
83+
assert.strictEqual(myVfs.readFileSync('/dst/a.txt', 'utf8'), 'a');
84+
}
85+
86+
// Overwriting a file drops one of its links
87+
{
88+
const myVfs = vfs.create();
89+
myVfs.writeFileSync('/a.txt', 'a');
90+
myVfs.writeFileSync('/b.txt', 'b');
91+
myVfs.linkSync('/b.txt', '/b-link.txt');
92+
assert.strictEqual(myVfs.statSync('/b-link.txt').nlink, 2);
93+
94+
myVfs.renameSync('/a.txt', '/b.txt');
95+
assert.strictEqual(myVfs.statSync('/b-link.txt').nlink, 1);
96+
assert.strictEqual(myVfs.readFileSync('/b-link.txt', 'utf8'), 'b');
97+
}
98+
99+
// Renaming a path onto itself is a no-op
100+
{
101+
const myVfs = vfs.create();
102+
myVfs.writeFileSync('/a.txt', 'a');
103+
myVfs.mkdirSync('/d');
104+
myVfs.writeFileSync('/d/keep.txt', 'keep');
105+
106+
myVfs.renameSync('/a.txt', '/a.txt');
107+
assert.strictEqual(myVfs.readFileSync('/a.txt', 'utf8'), 'a');
108+
assert.strictEqual(myVfs.statSync('/a.txt').nlink, 1);
109+
110+
myVfs.renameSync('/d', '/d');
111+
assert.strictEqual(myVfs.readFileSync('/d/keep.txt', 'utf8'), 'keep');
112+
}
113+
114+
// Renaming a hard link onto another link to the same file is a no-op
115+
{
116+
const myVfs = vfs.create();
117+
myVfs.writeFileSync('/a.txt', 'a');
118+
myVfs.linkSync('/a.txt', '/b.txt');
119+
120+
myVfs.renameSync('/a.txt', '/b.txt');
121+
assert.strictEqual(myVfs.existsSync('/a.txt'), true);
122+
assert.strictEqual(myVfs.existsSync('/b.txt'), true);
123+
assert.strictEqual(myVfs.statSync('/a.txt').nlink, 2);
124+
}

0 commit comments

Comments
 (0)