Skip to content

fs: rmSync() deletes a symlinked directory's contents then throws ENOTDIR when the path has a trailing slash #67186

Open

Description

@yohannesjoya

Version

v28.0.0-pre (main, 8d514c7), v22.20.0 for comparison

Platform

Linux 7.0.0-38-generic x86_64

Subsystem

fs

What steps will reproduce the bug?

link is a symlink to a directory. The path passed to rm ends with a slash.

const fs = require('node:fs');
const path = require('node:path');
const os = require('node:os');

function setup() {
  const d = fs.mkdtempSync(path.join(os.tmpdir(), 'rm-'));
  fs.mkdirSync(path.join(d, 'dir'));
  fs.writeFileSync(path.join(d, 'dir/keep.txt'), 'x');
  fs.symlinkSync('dir', path.join(d, 'link'));
  return d;
}

function report(label, d, err) {
  console.log(label, err ? err.code : 'ok',
    '| link:', fs.existsSync(path.join(d, 'link')),
    'dir:', fs.existsSync(path.join(d, 'dir')),
    'dir/keep.txt:', fs.existsSync(path.join(d, 'dir/keep.txt')));
}

let d = setup();
try { fs.rmSync(path.join(d, 'link/'), { recursive: true }); report('rmSync', d); }
catch (err) { report('rmSync', d, err); }

d = setup();
fs.rm(path.join(d, 'link/'), { recursive: true }, (err) => report('rm', d, err));

How often does it reproduce? Is there a required condition?

Every time. Requires a symlink to a directory, a trailing slash on the path, and recursive: true.

What is the expected behavior?

rmSync(), rm() and fsPromises.rm() should do the same thing. Whatever that is, a call that throws should not leave the tree half deleted.

What do you see instead?

On main:

rmSync ENOTDIR | link: true dir: true dir/keep.txt: false
rm ok | link: true dir: true dir/keep.txt: true

rmSync() deletes everything inside the target directory, then throws ENOTDIR. The symlink and the directory itself survive.

rm() and fsPromises.rm() report success and delete nothing.

On v22.20.0, rmSync() throws ENOTDIR and deletes nothing, so the sync side regressed when rmSync moved to C++ in #53617 (7168295).

Additional information

Why the three behaviors differ:

  • rmSync() is std::filesystem::remove_all(). The trailing slash makes lstat() follow the link, so the path reports as a directory. remove_all opens the directory, unlinks each entry, then calls unlink("link/"), which fails with ENOTDIR. By then the entries are gone.

    statx("/tmp/rm-x/link/", AT_SYMLINK_NOFOLLOW) = 0  (S_IFDIR)
    openat("/tmp/rm-x/link/", O_RDONLY|O_NOFOLLOW|O_DIRECTORY) = 20
    unlinkat(20, "keep.txt", 0) = 0
    unlink("/tmp/rm-x/link/") = -1 ENOTDIR
    
  • rm() and fsPromises.rm() use internal/fs/rimraf. lstat() also reports a directory, so _rmdir() calls rmdir("link/"), which fails with ENOTDIR. _rmdir() turns ENOTDIR into callback(originalErr), and originalErr is null on this path, so the call succeeds without touching anything.

POSIX rm -rf link/ deletes the directory contents, leaves the link and the directory, and exits 0. So the sync behavior matches the shell except for the error at the end.

#66673 normalizes dot segments but keeps a trailing slash, so it does not change this case.

I am happy to send a PR once there is agreement on which behavior is wanted: match rm -rf (delete contents, succeed), or treat the path as the symlink and refuse with ENOTDIR before deleting anything.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions