Skip to content

Commit ef09754

Browse files
committed
src: fix cpSyncCopyDir to dereference symlinks when dereference option 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: #59168
1 parent 61102cd commit ef09754

File tree

2 files changed

+88
-5
lines changed

2 files changed

+88
-5
lines changed

src/node_file.cc

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3595,16 +3595,43 @@ static void CpSyncCopyDir(const FunctionCallbackInfo<Value>& args) {
35953595
}
35963596
auto symlink_target_absolute = std::filesystem::weakly_canonical(
35973597
std::filesystem::absolute(src / symlink_target));
3598-
if (dir_entry.is_directory()) {
3598+
if (dereference) {
3599+
// When dereference is true, copy the actual content the symlink
3600+
// points to rather than creating a new symlink at the destination.
3601+
error.clear();
3602+
if (std::filesystem::is_directory(symlink_target_absolute, error)) {
3603+
std::filesystem::create_directory(dest_file_path, error);
3604+
if (error) {
3605+
env->ThrowStdErrException(error, "cp", dest_str.c_str());
3606+
return false;
3607+
}
3608+
auto success =
3609+
copy_dir_contents(symlink_target_absolute, dest_file_path);
3610+
if (!success) return false;
3611+
} else {
3612+
error.clear();
3613+
std::filesystem::copy_file(
3614+
symlink_target_absolute, dest_file_path, file_copy_opts,
3615+
error);
3616+
if (error) {
3617+
env->ThrowStdErrException(error, "cp", dest_str.c_str());
3618+
return false;
3619+
}
3620+
}
3621+
} else if (dir_entry.is_directory()) {
35993622
std::filesystem::create_directory_symlink(
36003623
symlink_target_absolute, dest_file_path, error);
3624+
if (error) {
3625+
env->ThrowStdErrException(error, "cp", dest_str.c_str());
3626+
return false;
3627+
}
36013628
} else {
36023629
std::filesystem::create_symlink(
36033630
symlink_target_absolute, dest_file_path, error);
3604-
}
3605-
if (error) {
3606-
env->ThrowStdErrException(error, "cp", dest_str.c_str());
3607-
return false;
3631+
if (error) {
3632+
env->ThrowStdErrException(error, "cp", dest_str.c_str());
3633+
return false;
3634+
}
36083635
}
36093636
}
36103637
} else if (dir_entry.is_directory()) {
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Refs: https://github.com/nodejs/node/issues/59168
2+
//
3+
// When cpSync() is called with {dereference: true}, symlinks *inside* the
4+
// source directory should be resolved and their targets copied as real
5+
// files/directories, not as new symlinks. This was broken in CpSyncCopyDir()
6+
// which always created symlinks regardless of the dereference option.
7+
8+
import { mustNotMutateObjectDeep } from '../common/index.mjs';
9+
import { nextdir } from '../common/fs.js';
10+
import assert from 'node:assert';
11+
import {
12+
cpSync,
13+
lstatSync,
14+
mkdirSync,
15+
readFileSync,
16+
symlinkSync,
17+
writeFileSync,
18+
} from 'node:fs';
19+
import { join } from 'node:path';
20+
import tmpdir from '../common/tmpdir.js';
21+
22+
tmpdir.refresh();
23+
24+
// Build source tree:
25+
// src/
26+
// src/real-file.txt (regular file)
27+
// src/link-to-file.txt --> src/real-file.txt (symlink to file)
28+
// src/real-subdir/
29+
// src/real-subdir/inner.txt (regular file inside subdirectory)
30+
// src/link-to-dir/ --> src/real-subdir/ (symlink to directory)
31+
32+
const src = nextdir();
33+
const dest = nextdir();
34+
35+
mkdirSync(src, mustNotMutateObjectDeep({ recursive: true }));
36+
writeFileSync(join(src, 'real-file.txt'), 'hello', 'utf8');
37+
symlinkSync(join(src, 'real-file.txt'), join(src, 'link-to-file.txt'));
38+
39+
const realSubdir = join(src, 'real-subdir');
40+
mkdirSync(realSubdir);
41+
writeFileSync(join(realSubdir, 'inner.txt'), 'inner', 'utf8');
42+
symlinkSync(realSubdir, join(src, 'link-to-dir'));
43+
44+
cpSync(src, dest, mustNotMutateObjectDeep({ dereference: true, recursive: true }));
45+
46+
// Symlinked file should have been dereferenced: copied as a real file.
47+
const linkToFileStat = lstatSync(join(dest, 'link-to-file.txt'));
48+
assert(!linkToFileStat.isSymbolicLink(), 'link-to-file.txt should not be a symlink in dest');
49+
assert(linkToFileStat.isFile(), 'link-to-file.txt should be a regular file in dest');
50+
assert.strictEqual(readFileSync(join(dest, 'link-to-file.txt'), 'utf8'), 'hello');
51+
52+
// Symlinked directory should have been dereferenced: copied as a real directory.
53+
const linkToDirStat = lstatSync(join(dest, 'link-to-dir'));
54+
assert(!linkToDirStat.isSymbolicLink(), 'link-to-dir should not be a symlink in dest');
55+
assert(linkToDirStat.isDirectory(), 'link-to-dir should be a regular directory in dest');
56+
assert.strictEqual(readFileSync(join(dest, 'link-to-dir', 'inner.txt'), 'utf8'), 'inner');

0 commit comments

Comments
 (0)