From 73171e829ad316b82749f1d76e18e6849c67adf1 Mon Sep 17 00:00:00 2001 From: leotwang Date: Tue, 31 Mar 2026 13:20:09 +0800 Subject: [PATCH 1/2] fs: use libuv scandir for cpSync directory iteration Replace std::filesystem::directory_iterator with libuv's uv_fs_scandir to avoid process abort when copying directories with non-ASCII paths. Previously, std::filesystem::directory_iterator could throw uncatchable exceptions when encountering encoding issues (e.g., GBK-encoded Chinese paths on Windows). In environments with buggy libc++ implementations (especially Electron), this triggers __libcpp_verbose_abort(), causing immediate process termination that cannot be caught by JavaScript try-catch blocks. This patch uses libuv's uv_fs_scandir and uv_fs_scandir_next instead, which properly handle path encoding and return errors that can be converted to JavaScript exceptions. This approach is consistent with other fs operations in Node.js (e.g., fs.readdirSync) and ensures: 1. No process abort on encoding errors 2. Errors are catchable by JavaScript 3. fs.cpSync() can successfully copy directories with non-ASCII paths 4. Consistent behavior across different C++ standard library implementations The fix also adds proper cleanup of uv_fs_t requests on all error paths to prevent resource leaks. --- src/node_file.cc | 77 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 64 insertions(+), 13 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index 7fb87639b37e96..f4f38223b18a4c 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3738,23 +3738,64 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { &isolate](std::filesystem::path src, std::filesystem::path dest) { std::error_code error; - for (auto dir_entry : std::filesystem::directory_iterator(src)) { - auto dest_file_path = dest / dir_entry.path().filename(); + + // Use libuv's uv_fs_scandir instead of std::filesystem::directory_iterator + // to avoid crashes with non-ASCII paths (especially GBK-encoded paths on Windows + // when running inside Electron with a buggy libc++ implementation). + auto src_str = ConvertPathToUTF8(src); + uv_fs_t scandir_req; + int rc = uv_fs_scandir(nullptr, &scandir_req, src_str.c_str(), 0, nullptr); + if (rc < 0) { + env->ThrowUVException(rc, "scandir", src_str.c_str()); + uv_fs_req_cleanup(&scandir_req); + return false; + } + + uv_dirent_t ent; + while ((rc = uv_fs_scandir_next(&scandir_req, &ent)) != UV_EOF) { + if (rc < 0) { + env->ThrowUVException(rc, "scandir", src_str.c_str()); + uv_fs_req_cleanup(&scandir_req); + return false; + } + + auto entry_name = std::filesystem::path(ent.name); + auto entry_path = src / entry_name; + auto dest_file_path = dest / entry_name; auto dest_str = ConvertPathToUTF8(dest); - - if (dir_entry.is_symlink()) { + + // Get entry type using uv_fs_lstat + uv_fs_t stat_req; + auto entry_path_str = ConvertPathToUTF8(entry_path); + rc = uv_fs_lstat(nullptr, &stat_req, entry_path_str.c_str(), nullptr); + if (rc < 0) { + env->ThrowUVException(rc, "lstat", entry_path_str.c_str()); + uv_fs_req_cleanup(&stat_req); + uv_fs_req_cleanup(&scandir_req); + return false; + } + + const uv_stat_t* stat = static_cast(stat_req.ptr); + bool is_symlink = S_ISLNK(stat->st_mode); + bool is_directory = S_ISDIR(stat->st_mode); + bool is_regular = S_ISREG(stat->st_mode); + uv_fs_req_cleanup(&stat_req); + + if (is_symlink) { if (verbatim_symlinks) { std::filesystem::copy_symlink( - dir_entry.path(), dest_file_path, error); + entry_path, dest_file_path, error); if (error) { env->ThrowStdErrException(error, "cp", dest_str.c_str()); + uv_fs_req_cleanup(&scandir_req); return false; } } else { auto symlink_target = - std::filesystem::read_symlink(dir_entry.path().c_str(), error); + std::filesystem::read_symlink(entry_path.c_str(), error); if (error) { env->ThrowStdErrException(error, "cp", dest_str.c_str()); + uv_fs_req_cleanup(&scandir_req); return false; } @@ -3764,6 +3805,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { std::filesystem::read_symlink(dest_file_path.c_str(), error); if (error) { env->ThrowStdErrException(error, "cp", dest_str.c_str()); + uv_fs_req_cleanup(&scandir_req); return false; } @@ -3774,6 +3816,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { "Cannot copy %s to a subdirectory of self %s"; THROW_ERR_FS_CP_EINVAL( env, message, symlink_target, current_dest_symlink_target); + uv_fs_req_cleanup(&scandir_req); return false; } @@ -3786,6 +3829,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { "cannot overwrite %s with %s"; THROW_ERR_FS_CP_SYMLINK_TO_SUBDIRECTORY( env, message, current_dest_symlink_target, symlink_target); + uv_fs_req_cleanup(&scandir_req); return false; } @@ -3795,6 +3839,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { std::filesystem::remove(dest_file_path, error); if (error) { env->ThrowStdErrException(error, "cp", dest_str.c_str()); + uv_fs_req_cleanup(&scandir_req); return false; } } else if (std::filesystem::is_regular_file(dest_file_path)) { @@ -3804,13 +3849,14 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { std::make_error_code(std::errc::file_exists), "cp", dest_file_path_str.c_str()); + uv_fs_req_cleanup(&scandir_req); return false; } } } auto symlink_target_absolute = std::filesystem::weakly_canonical( std::filesystem::absolute(src / symlink_target)); - if (dir_entry.is_directory()) { + if (is_directory) { std::filesystem::create_directory_symlink( symlink_target_absolute, dest_file_path, error); } else { @@ -3819,37 +3865,42 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { } if (error) { env->ThrowStdErrException(error, "cp", dest_str.c_str()); + uv_fs_req_cleanup(&scandir_req); return false; } } - } else if (dir_entry.is_directory()) { - auto entry_dir_path = src / dir_entry.path().filename(); + } else if (is_directory) { std::filesystem::create_directory(dest_file_path); - auto success = copy_dir_contents(entry_dir_path, dest_file_path); + auto success = copy_dir_contents(entry_path, dest_file_path); if (!success) { + uv_fs_req_cleanup(&scandir_req); return false; } - } else if (dir_entry.is_regular_file()) { + } else if (is_regular) { std::filesystem::copy_file( - dir_entry.path(), dest_file_path, file_copy_opts, error); + entry_path, dest_file_path, file_copy_opts, error); if (error) { if (error.value() == EEXIST) { THROW_ERR_FS_CP_EEXIST(isolate, "[ERR_FS_CP_EEXIST]: Target already exists: " "cp returned EEXIST (%s already exists)", dest_file_path); + uv_fs_req_cleanup(&scandir_req); return false; } env->ThrowStdErrException(error, "cp", dest_str.c_str()); + uv_fs_req_cleanup(&scandir_req); return false; } if (preserve_timestamps && - !CopyUtimes(dir_entry.path(), dest_file_path, env)) { + !CopyUtimes(entry_path, dest_file_path, env)) { + uv_fs_req_cleanup(&scandir_req); return false; } } } + uv_fs_req_cleanup(&scandir_req); return true; }; From b2309bc43721128514596b1cb01f4157eaf77e74 Mon Sep 17 00:00:00 2001 From: leotwang Date: Tue, 31 Mar 2026 13:20:17 +0800 Subject: [PATCH 2/2] test: add cpSync error handling test Add test to ensure fs.cpSync properly handles directory iteration errors without causing process abort. --- .../parallel/test-fs-cpsync-error-handling.js | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 test/parallel/test-fs-cpsync-error-handling.js diff --git a/test/parallel/test-fs-cpsync-error-handling.js b/test/parallel/test-fs-cpsync-error-handling.js new file mode 100644 index 00000000000000..9dda8fefc5dcca --- /dev/null +++ b/test/parallel/test-fs-cpsync-error-handling.js @@ -0,0 +1,86 @@ +'use strict'; + +// Test that fs.cpSync properly handles directory iteration errors +// instead of causing process abort. +// +// This test ensures that when directory_iterator construction fails +// (e.g., due to permission issues or malformed paths), the error +// is properly converted to a JavaScript exception rather than +// triggering std::terminate or __libcpp_verbose_abort. + +const common = require('../common'); +const { cpSync, mkdirSync, writeFileSync } = require('fs'); +const { join } = require('path'); +const assert = require('assert'); +const tmpdir = require('../common/tmpdir'); + +tmpdir.refresh(); + +// Test 1: Copying from non-existent directory should throw, not abort +{ + const nonExistent = join(tmpdir.path, 'does-not-exist'); + const dest = join(tmpdir.path, 'dest1'); + + assert.throws( + () => cpSync(nonExistent, dest, { recursive: true }), + { + code: 'ENOENT', + message: /ENOENT/ + }, + 'cpSync should throw ENOENT for non-existent source directory' + ); +} + +// Test 2: Copying directory with unreadable subdirectory +// (This test is platform-specific and may be skipped on Windows) +if (!common.isWindows) { + const srcDir = join(tmpdir.path, 'src-unreadable'); + const unreadableSubdir = join(srcDir, 'unreadable'); + const dest = join(tmpdir.path, 'dest2'); + + mkdirSync(srcDir); + mkdirSync(unreadableSubdir); + writeFileSync(join(unreadableSubdir, 'file.txt'), 'content'); + + try { + require('fs').chmodSync(unreadableSubdir, 0o000); + + // Should throw error, not abort + assert.throws( + () => cpSync(srcDir, dest, { recursive: true }), + { + code: 'EACCES', + }, + 'cpSync should throw EACCES for unreadable directory' + ); + } finally { + // Restore permissions for cleanup + require('fs').chmodSync(unreadableSubdir, 0o755); + } +} + +// Test 3: Basic successful copy to ensure fix doesn't break normal operation +{ + const srcDir = join(tmpdir.path, 'src-normal'); + const dest = join(tmpdir.path, 'dest-normal'); + + mkdirSync(srcDir); + writeFileSync(join(srcDir, 'file1.txt'), 'content1'); + mkdirSync(join(srcDir, 'subdir')); + writeFileSync(join(srcDir, 'subdir', 'file2.txt'), 'content2'); + + // Should not throw + cpSync(srcDir, dest, { recursive: true }); + + const fs = require('fs'); + assert.ok(fs.existsSync(join(dest, 'file1.txt'))); + assert.ok(fs.existsSync(join(dest, 'subdir', 'file2.txt'))); + assert.strictEqual( + fs.readFileSync(join(dest, 'file1.txt'), 'utf8'), + 'content1' + ); + assert.strictEqual( + fs.readFileSync(join(dest, 'subdir', 'file2.txt'), 'utf8'), + 'content2' + ); +}