Skip to content

vfs: fix rename over non-empty directory - #65613

Open
christianaurichzm wants to merge 1 commit into
nodejs:mainfrom
christianaurichzm:vfs-fix-rename-over-non-empty-directory
Open

vfs: fix rename over non-empty directory#65613
christianaurichzm wants to merge 1 commit into
nodejs:mainfrom
christianaurichzm:vfs-fix-rename-over-non-empty-directory

Conversation

@christianaurichzm

Copy link
Copy Markdown
Contributor

MemoryProvider#renameSync() only rejected a destination whose type differed from the source (EISDIR/ENOTDIR). When both sides were directories it fell through to replacing the destination in its parent, so the destination directory and everything under it disappeared:

// node --experimental-vfs
const vfs = require('node:vfs').create();
vfs.mkdirSync('/src');
vfs.mkdirSync('/dst');
vfs.writeFileSync('/dst/keep.txt', 'keep');

vfs.renameSync('/src', '/dst');   // no error
vfs.existsSync('/dst/keep.txt');  // false

rename(2) accepts an existing directory as the destination only when it is empty, and fails with ENOTEMPTY otherwise. RealFSProvider delegates to fs.renameSync() and already behaves that way, so the two providers backing the same node:vfs API disagreed.

Two smaller fixes in the same function, both covered by the new tests:

  • a file replaced by a rename now loses a link, so a hard link to it no longer reports an inflated nlink;
  • a rename whose two names resolve to the same entry is a no-op; renaming one hard link onto another used to remove the source name while the file kept its old link count.

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>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. vfs Issues and PRs related to the virtual filesystem subsystem. labels Aug 28, 2026
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.07%. Comparing base (9baabd4) to head (e9333ce).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65613      +/-   ##
==========================================
- Coverage   90.07%   90.07%   -0.01%     
==========================================
  Files         751      751              
  Lines      254875   254928      +53     
  Branches    48108    48126      +18     
==========================================
+ Hits       229579   229624      +45     
- Misses      16466    16484      +18     
+ Partials     8830     8820      -10     
Files with missing lines Coverage Δ
lib/internal/vfs/providers/memory.js 95.18% <100.00%> (+0.05%) ⬆️

... and 39 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. vfs Issues and PRs related to the virtual filesystem subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants