From 77ca5ca075f2ba67bbcbcd135b99bd4516ed8863 Mon Sep 17 00:00:00 2001 From: Jason Zhang Date: Fri, 20 Sep 2024 14:16:47 +0930 Subject: [PATCH] fs: convert to u8 string for filesystem path PR-URL: https://github.com/nodejs/node/pull/54653 Fixes: https://github.com/nodejs/node/issues/54476 Reviewed-By: Yagiz Nizipli Reviewed-By: Antoine du Hamel Reviewed-By: Joyee Cheung Reviewed-By: James M Snell --- src/node_file.cc | 77 +++++++++++-------- src/util.h | 4 + .../index.js" | 3 + test/parallel/test-fs-cp.mjs | 8 ++ 4 files changed, 61 insertions(+), 31 deletions(-) create mode 100644 "test/fixtures/copy/utf/\346\226\260\345\273\272\346\226\207\344\273\266\345\244\271/index.js" diff --git a/src/node_file.cc b/src/node_file.cc index f569eea1a7f9b8..7554a7b3b696b0 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3130,15 +3130,16 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { ToNamespacedPath(env, &src); THROW_IF_INSUFFICIENT_PERMISSIONS( env, permission::PermissionScope::kFileSystemRead, src.ToStringView()); - auto src_path = std::filesystem::path(src.ToStringView()); + + auto src_path = std::filesystem::path(src.ToU8StringView()); BufferValue dest(isolate, args[1]); CHECK_NOT_NULL(*dest); ToNamespacedPath(env, &dest); THROW_IF_INSUFFICIENT_PERMISSIONS( env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView()); - auto dest_path = std::filesystem::path(dest.ToStringView()); + auto dest_path = std::filesystem::path(dest.ToU8StringView()); bool dereference = args[2]->IsTrue(); bool recursive = args[3]->IsTrue(); @@ -3146,6 +3147,7 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { auto src_status = dereference ? std::filesystem::symlink_status(src_path, error_code) : std::filesystem::status(src_path, error_code); + if (error_code) { #ifdef _WIN32 int errorno = uv_translate_sys_error(error_code.value()); @@ -3169,34 +3171,41 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { if (!error_code) { // Check if src and dest are identical. if (std::filesystem::equivalent(src_path, dest_path)) { - std::string message = - "src and dest cannot be the same " + dest_path.string(); - return THROW_ERR_FS_CP_EINVAL(env, message.c_str()); + std::u8string message = + u8"src and dest cannot be the same " + dest_path.u8string(); + return THROW_ERR_FS_CP_EINVAL( + env, reinterpret_cast(message.c_str())); } const bool dest_is_dir = dest_status.type() == std::filesystem::file_type::directory; if (src_is_dir && !dest_is_dir) { - std::string message = "Cannot overwrite non-directory " + - src_path.string() + " with directory " + - dest_path.string(); - return THROW_ERR_FS_CP_DIR_TO_NON_DIR(env, message.c_str()); + std::u8string message = u8"Cannot overwrite non-directory " + + src_path.u8string() + u8" with directory " + + dest_path.u8string(); + return THROW_ERR_FS_CP_DIR_TO_NON_DIR( + env, reinterpret_cast(message.c_str())); } if (!src_is_dir && dest_is_dir) { - std::string message = "Cannot overwrite directory " + dest_path.string() + - " with non-directory " + src_path.string(); - return THROW_ERR_FS_CP_NON_DIR_TO_DIR(env, message.c_str()); + std::u8string message = u8"Cannot overwrite directory " + + dest_path.u8string() + u8" with non-directory " + + src_path.u8string(); + return THROW_ERR_FS_CP_NON_DIR_TO_DIR( + env, reinterpret_cast(message.c_str())); } } - std::string dest_path_str = dest_path.string(); + std::u8string dest_path_str = dest_path.u8string(); + // Check if dest_path is a subdirectory of src_path. - if (src_is_dir && dest_path_str.starts_with(src_path.string())) { - std::string message = "Cannot copy " + src_path.string() + - " to a subdirectory of self " + dest_path.string(); - return THROW_ERR_FS_CP_EINVAL(env, message.c_str()); + if (src_is_dir && dest_path_str.starts_with(src_path.u8string())) { + std::u8string message = u8"Cannot copy " + src_path.u8string() + + u8" to a subdirectory of self " + + dest_path.u8string(); + return THROW_ERR_FS_CP_EINVAL( + env, reinterpret_cast(message.c_str())); } auto dest_parent = dest_path.parent_path(); @@ -3207,9 +3216,11 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { dest_parent.parent_path() != dest_parent) { if (std::filesystem::equivalent( src_path, dest_path.parent_path(), error_code)) { - std::string message = "Cannot copy " + src_path.string() + - " to a subdirectory of self " + dest_path.string(); - return THROW_ERR_FS_CP_EINVAL(env, message.c_str()); + std::u8string message = u8"Cannot copy " + src_path.u8string() + + u8" to a subdirectory of self " + + dest_path.u8string(); + return THROW_ERR_FS_CP_EINVAL( + env, reinterpret_cast(message.c_str())); } // If equivalent fails, it's highly likely that dest_parent does not exist @@ -3221,25 +3232,29 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { } if (src_is_dir && !recursive) { - std::string message = - "Recursive option not enabled, cannot copy a directory: " + - src_path.string(); - return THROW_ERR_FS_EISDIR(env, message.c_str()); + std::u8string message = + u8"Recursive option not enabled, cannot copy a directory: " + + src_path.u8string(); + return THROW_ERR_FS_EISDIR(env, + reinterpret_cast(message.c_str())); } switch (src_status.type()) { case std::filesystem::file_type::socket: { - std::string message = "Cannot copy a socket file: " + dest_path.string(); - return THROW_ERR_FS_CP_SOCKET(env, message.c_str()); + std::u8string message = u8"Cannot copy a socket file: " + dest_path_str; + return THROW_ERR_FS_CP_SOCKET( + env, reinterpret_cast(message.c_str())); } case std::filesystem::file_type::fifo: { - std::string message = "Cannot copy a FIFO pipe: " + dest_path.string(); - return THROW_ERR_FS_CP_FIFO_PIPE(env, message.c_str()); + std::u8string message = u8"Cannot copy a FIFO pipe: " + dest_path_str; + return THROW_ERR_FS_CP_FIFO_PIPE( + env, reinterpret_cast(message.c_str())); } case std::filesystem::file_type::unknown: { - std::string message = - "Cannot copy an unknown file type: " + dest_path.string(); - return THROW_ERR_FS_CP_UNKNOWN(env, message.c_str()); + std::u8string message = + u8"Cannot copy an unknown file type: " + dest_path_str; + return THROW_ERR_FS_CP_UNKNOWN( + env, reinterpret_cast(message.c_str())); } default: break; diff --git a/src/util.h b/src/util.h index a6da8720c499df..cbfe20c811901e 100644 --- a/src/util.h +++ b/src/util.h @@ -562,6 +562,10 @@ class BufferValue : public MaybeStackBuffer { inline std::string_view ToStringView() const { return std::string_view(out(), length()); } + inline std::u8string_view ToU8StringView() const { + return std::u8string_view(reinterpret_cast(out()), + length()); + } }; #define SPREAD_BUFFER_ARG(val, name) \ diff --git "a/test/fixtures/copy/utf/\346\226\260\345\273\272\346\226\207\344\273\266\345\244\271/index.js" "b/test/fixtures/copy/utf/\346\226\260\345\273\272\346\226\207\344\273\266\345\244\271/index.js" new file mode 100644 index 00000000000000..12388b0457bdda --- /dev/null +++ "b/test/fixtures/copy/utf/\346\226\260\345\273\272\346\226\207\344\273\266\345\244\271/index.js" @@ -0,0 +1,3 @@ +module.exports = { + purpose: 'testing copy' +}; diff --git a/test/parallel/test-fs-cp.mjs b/test/parallel/test-fs-cp.mjs index 16a52bc12c49f1..54bb3f95bf40d5 100644 --- a/test/parallel/test-fs-cp.mjs +++ b/test/parallel/test-fs-cp.mjs @@ -30,6 +30,14 @@ function nextdir() { // Synchronous implementation of copy. +// It copies a nested folder containing UTF characters. +{ + const src = './test/fixtures/copy/utf/新建文件夹'; + const dest = nextdir(); + cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })); + assertDirEquivalent(src, dest); +} + // It copies a nested folder structure with files and folders. { const src = './test/fixtures/copy/kitchen-sink';