src: fix cpSyncCopyDir to dereference symlinks when dereference option is set#62402
src: fix cpSyncCopyDir to dereference symlinks when dereference option is set#62402crawfordxx wants to merge 1 commit intonodejs:mainfrom
Conversation
…n is set
When fs.cpSync() is called with {dereference: true}, symlinks in the source
directory should be followed and their targets copied to the destination
instead of creating new symlinks. This behavior was correctly implemented in
the JavaScript version of cpSync, but was lost when the inner copyDir loop
was migrated to C++ in CpSyncCopyDir().
The dereference parameter was captured but only used for error-condition
checks, not to decide whether to create a symlink or copy the actual content.
Fix by checking dereference before creating symlinks: when true, copy the
actual file (using copy_file) or recurse into the actual directory
(using copy_dir_contents) that the symlink points to.
Fixes: nodejs#59168
themavik
left a comment
There was a problem hiding this comment.
The dereference branch in CpSyncCopyDir now copies symlink targets via copy_dir_contents / copy_file instead of recreating links — correct behavior. nit: confirm filters (should_copy_signal / dirent_filter) still apply the same when the walk hits dereferenced symlinks as for ordinary entries.
|
Thanks for the review @themavik! Regarding the filters question: The |
Summary
When
fs.cpSync()orfs.promises.cp()is called with{dereference: true},symlinks in the source directory should be followed and their targets copied
to the destination as real files/directories. This behavior was correctly
implemented in the original JavaScript version of
cpSync, but was lost whenthe inner directory-copy loop was migrated to C++ in PR #58461.
Root Cause
In
CpSyncCopyDir(src/node_file.cc), thedereferenceparameter wascaptured by the lambda but only used for error-condition checks:
At the end of the
is_symlink()branch, a new symlink was always createdat the destination regardless of whether
dereferencewas true:Fix
When
dereferenceis true and we encounter a symlink:copy_dir_contents()copy_file()When
dereferenceis false (the default), behavior is unchanged — a newsymlink is created at the destination.
Test
Existing test case in
test/parallel/test-fs-cp.mjscovers thedereferenceoption. The specific scenario from #59168 (symlink to a file outside the source
directory with
{dereference: true, recursive: true}) now produces a regularfile at the destination instead of a symlink.
Fixes: #59168