Repository navigation
fs: normalize paths with dot segments in rm and rmSync - #66472
RohithPariki wants to merge 1 commit into
Conversation
|
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. Caution AgentScan found account activity patterns that may be consistent with automation. This is a heuristic, not proof that this pull request was opened by an agent or violates policy. AI-assisted contributions are permitted, but automated tooling must not open pull requests without advance approval, and contributors must personally understand, test, verify, and take responsibility for every submitted change. See the AgentScan analysis, AI use policy, and automation policy for additional context. |
|
For a first-time contribution, we recommend focusing on one PR at a time. Could you close one of these PRs for now? |
When paths containing '.' or '..' components (e.g., 'a/b/../.') are passed to fs.rm(), fs.rmSync(), or fs.promises.rm(), the different codepaths (native binding.rmSync vs JS rimraf) could produce inconsistent results because the path was used as-is without normalization. Add pathModule.normalize() after getValidatedPath() in all three rm variants to ensure dot-segment components are resolved before the path reaches the underlying removal implementation. Fixes: nodejs#61958 Signed-off-by: Rohith Pariki <[email protected]>
0ee0e2e to
0dc372c
Compare
|
Superseded by #66673. |
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
fs
Description of change
When paths containing relative components (such as
.or..) are passed tofs.rm(),fs.rmSync(), orfs.promises.rm(), the different underlying implementations (binding.rmSyncviastd::filesystem::remove_allvs JSrimraf) produce divergent results:rimraf, child paths are constructed by concatenating onto the unresolved path. When recursion removes intermediate directory components, subsequent calls fail withENOENT, whichrimraftreats as "already deleted," causing silent partial removals.rmSyncthrows an error without a.codeproperty on paths with trailing/..This PR normalizes dot segments before reaching the underlying removal routines:
normalizePathForRm()inlib/fs.jsandlib/internal/fs/promises.js.Bufferinputs: forBufferpaths, binary byte content is preserved 1:1 usinglatin1round-tripping before and after normalization to prevent data corruption on non-UTF-8 paths.URLinputs are converted to strings viagetValidatedPath()before normalization.Tests
Added regression tests in
test/parallel/test-fs-rm.jscovering:a/b/../.,a/b/..,a/.,a/b/c/.,a/b/c/../..,a/b/c/d/../../..,a/b/.././b,./a) acrossfs.rmSync,fs.rm(callback), andfs.promises.rm.Fixes: #61958