Skip to content

fs: normalize dot segments in rm and rmSync - #66673

Open
RohithPariki wants to merge 1 commit into
nodejs:mainfrom
RohithPariki:fix/fs-rm-dot-segments
Open

RohithPariki wants to merge 1 commit into
nodejs:mainfrom
RohithPariki:fix/fs-rm-dot-segments

Conversation

@RohithPariki

Copy link
Copy Markdown
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

fs

Description of change

When paths containing relative dot segments (such as . or ..) are passed to fs.rm(), fs.rmSync(), or fs.promises.rm(), the different underlying implementations (binding.rmSync via std::filesystem::remove_all vs JavaScript rimraf) produce divergent results:

  • In rimraf, child paths are constructed by concatenating onto the unresolved path. When recursion removes intermediate directory components, subsequent calls fail with ENOENT, which rimraf treats as "already deleted," causing silent partial removals.
  • rmSync throws an error without a .code property on paths with trailing /..

This PR normalizes dot segments before reaching the underlying removal routines:

  • Introduces normalizePathForRm() in lib/fs.js and lib/internal/fs/promises.js.
  • Correctly handles both string and Buffer inputs: for Buffer paths, binary byte content is preserved 1:1 using latin1 round-tripping before and after normalization to prevent data corruption on non-UTF-8 paths.
  • WHATWG URL inputs are converted to strings via getValidatedPath() before normalization.
Tests

Added regression tests in test/parallel/test-fs-rm.js covering:

  • The full relative path matrix (a/b/../., a/b/.., a/., a/b/c/., a/b/c/../.., a/b/c/d/../../.., a/b/.././b, ./a) across fs.rmSync, fs.rm (callback), and fs.promises.rm.
  • Buffer paths across sync and promises implementations.
  • Non-UTF-8 binary Buffer paths to ensure byte preservation.

Fixes: #61958

When paths containing relative components (such as '.' or '..') are
passed to fs.rm(), fs.rmSync(), or fs.promises.rm(), the different
underlying implementations (native binding.rmSync vs JS rimraf)
can produce divergent results or silent deletion failures because
paths were used as-is without canonicalization.

Add normalizePathForRm to resolve dot-segment components for both
string and Buffer inputs using byte-safe latin1 preservation before
reaching the underlying deletion routines.

Fixes: nodejs#61958
Signed-off-by: Rohith Pariki <[email protected]>
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run. labels Oct 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

@MikeMcC399

Copy link
Copy Markdown
Contributor

You have an open unapproved PR #66471 and so you have already reached the limit of 1 open unapproved PR for a "first-time contributor".

The recommendations in:

advise you should tackle only one issue at a time and that you should not open any new PRs until your first PR has been approved.

You already acknowledged this rule in #66472 (comment)

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

Labels

fs Issues and PRs related to file-system APIs and the fs module. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discrepancy between fs.rmSync and fs/promises.rm behavior with . and .. in paths

3 participants