From e9333ce3258684ac1a70ece92baf1ad7cd5ddabd Mon Sep 17 00:00:00 2001 From: Christian Aurich Date: Fri, 28 Aug 2026 00:15:38 -0300 Subject: [PATCH] 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 --- lib/internal/vfs/providers/memory.js | 12 ++++++ test/parallel/test-vfs-rename.js | 64 ++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/lib/internal/vfs/providers/memory.js b/lib/internal/vfs/providers/memory.js index acec5b4166b2..dd240993afbb 100644 --- a/lib/internal/vfs/providers/memory.js +++ b/lib/internal/vfs/providers/memory.js @@ -840,6 +840,10 @@ class MemoryProvider extends VirtualProvider { // Check if destination exists const existingDest = newParent.children.get(newName); + if (existingDest === entry) { + // Both names resolve to the same entry: rename does nothing + return; + } if (existingDest) { // Cannot overwrite a directory with a non-directory if (existingDest.isDirectory() && !entry.isDirectory()) { @@ -849,6 +853,14 @@ class MemoryProvider extends VirtualProvider { if (!existingDest.isDirectory() && entry.isDirectory()) { throw createENOTDIR('rename', newPath); } + if (existingDest.isDirectory()) { + // Cannot overwrite a non-empty directory + if (existingDest.children.size > 0) { + throw createENOTEMPTY('rename', newPath); + } + } else { + existingDest.nlink--; + } } // Remove from old location (after destination validation) diff --git a/test/parallel/test-vfs-rename.js b/test/parallel/test-vfs-rename.js index 69daba43c11d..b1ca98dc684e 100644 --- a/test/parallel/test-vfs-rename.js +++ b/test/parallel/test-vfs-rename.js @@ -58,3 +58,67 @@ const vfs = require('node:vfs'); assert.strictEqual(myVfs.existsSync('/a/b/c'), false); assert.strictEqual(myVfs.readFileSync('/a/file.txt', 'utf8'), 'data'); } + +// Renaming a directory onto a non-empty directory throws ENOTEMPTY +{ + const myVfs = vfs.create(); + myVfs.mkdirSync('/src'); + myVfs.mkdirSync('/dst'); + myVfs.writeFileSync('/dst/keep.txt', 'keep'); + + assert.throws(() => myVfs.renameSync('/src', '/dst'), { code: 'ENOTEMPTY' }); + assert.strictEqual(myVfs.readFileSync('/dst/keep.txt', 'utf8'), 'keep'); + assert.strictEqual(myVfs.existsSync('/src'), true); +} + +// Renaming a directory onto an empty directory succeeds +{ + const myVfs = vfs.create(); + myVfs.mkdirSync('/src'); + myVfs.writeFileSync('/src/a.txt', 'a'); + myVfs.mkdirSync('/dst'); + + myVfs.renameSync('/src', '/dst'); + assert.strictEqual(myVfs.existsSync('/src'), false); + assert.strictEqual(myVfs.readFileSync('/dst/a.txt', 'utf8'), 'a'); +} + +// Overwriting a file drops one of its links +{ + const myVfs = vfs.create(); + myVfs.writeFileSync('/a.txt', 'a'); + myVfs.writeFileSync('/b.txt', 'b'); + myVfs.linkSync('/b.txt', '/b-link.txt'); + assert.strictEqual(myVfs.statSync('/b-link.txt').nlink, 2); + + myVfs.renameSync('/a.txt', '/b.txt'); + assert.strictEqual(myVfs.statSync('/b-link.txt').nlink, 1); + assert.strictEqual(myVfs.readFileSync('/b-link.txt', 'utf8'), 'b'); +} + +// Renaming a path onto itself is a no-op +{ + const myVfs = vfs.create(); + myVfs.writeFileSync('/a.txt', 'a'); + myVfs.mkdirSync('/d'); + myVfs.writeFileSync('/d/keep.txt', 'keep'); + + myVfs.renameSync('/a.txt', '/a.txt'); + assert.strictEqual(myVfs.readFileSync('/a.txt', 'utf8'), 'a'); + assert.strictEqual(myVfs.statSync('/a.txt').nlink, 1); + + myVfs.renameSync('/d', '/d'); + assert.strictEqual(myVfs.readFileSync('/d/keep.txt', 'utf8'), 'keep'); +} + +// Renaming a hard link onto another link to the same file is a no-op +{ + const myVfs = vfs.create(); + myVfs.writeFileSync('/a.txt', 'a'); + myVfs.linkSync('/a.txt', '/b.txt'); + + myVfs.renameSync('/a.txt', '/b.txt'); + assert.strictEqual(myVfs.existsSync('/a.txt'), true); + assert.strictEqual(myVfs.existsSync('/b.txt'), true); + assert.strictEqual(myVfs.statSync('/a.txt').nlink, 2); +}