From 56e5bd8d2acd3698547b9d18d16620ed4e11a99f Mon Sep 17 00:00:00 2001 From: jazelly Date: Mon, 7 Oct 2024 10:25:31 +1030 Subject: [PATCH] fs: use `wstring` on Windows paths Co-authored-by: Antoine du Hamel PR-URL: https://github.com/nodejs/node/pull/55171 Reviewed-By: Rafael Gonzaga --- src/node_file.cc | 115 +++++++++++++++++++++++------------ test/parallel/test-fs-cp.mjs | 2 +- 2 files changed, 76 insertions(+), 41 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index c69e0e0ea29..dfdc25fd1d8 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3127,6 +3127,55 @@ static void GetFormatOfExtensionlessFile( return args.GetReturnValue().Set(EXTENSIONLESS_FORMAT_JAVASCRIPT); } +#ifdef _WIN32 +std::wstring ConvertToWideString(const std::string& str) { + int size_needed = MultiByteToWideChar( + CP_UTF8, 0, &str[0], static_cast(str.size()), nullptr, 0); + std::wstring wstrTo(size_needed, 0); + MultiByteToWideChar(CP_UTF8, + 0, + &str[0], + static_cast(str.size()), + &wstrTo[0], + size_needed); + return wstrTo; +} + +#define BufferValueToPath(str) \ + std::filesystem::path(ConvertToWideString(str.ToString())) + +std::string ConvertWideToUTF8(const std::wstring& wstr) { + if (wstr.empty()) return std::string(); + + int size_needed = WideCharToMultiByte(CP_UTF8, + 0, + &wstr[0], + static_cast(wstr.size()), + nullptr, + 0, + nullptr, + nullptr); + std::string strTo(size_needed, 0); + WideCharToMultiByte(CP_UTF8, + 0, + &wstr[0], + static_cast(wstr.size()), + &strTo[0], + size_needed, + nullptr, + nullptr); + return strTo; +} + +#define PathToString(path) ConvertWideToUTF8(path.wstring()); + +#else // _WIN32 + +#define BufferValueToPath(str) std::filesystem::path(str.ToStringView()); +#define PathToString(path) path.native(); + +#endif // _WIN32 + static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Isolate* isolate = env->isolate(); @@ -3139,7 +3188,7 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { THROW_IF_INSUFFICIENT_PERMISSIONS( env, permission::PermissionScope::kFileSystemRead, src.ToStringView()); - auto src_path = std::filesystem::path(src.ToU8StringView()); + auto src_path = BufferValueToPath(src); BufferValue dest(isolate, args[1]); CHECK_NOT_NULL(*dest); @@ -3147,7 +3196,7 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { THROW_IF_INSUFFICIENT_PERMISSIONS( env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView()); - auto dest_path = std::filesystem::path(dest.ToU8StringView()); + auto dest_path = BufferValueToPath(dest); bool dereference = args[2]->IsTrue(); bool recursive = args[3]->IsTrue(); @@ -3176,47 +3225,41 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { (src_status.type() == std::filesystem::file_type::directory) || (dereference && src_status.type() == std::filesystem::file_type::symlink); + auto src_path_str = PathToString(src_path); + auto dest_path_str = PathToString(dest_path); + if (!error_code) { // Check if src and dest are identical. if (std::filesystem::equivalent(src_path, dest_path)) { - 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())); + std::string message = "src and dest cannot be the same %s"; + return THROW_ERR_FS_CP_EINVAL(env, message.c_str(), dest_path_str); } const bool dest_is_dir = dest_status.type() == std::filesystem::file_type::directory; - if (src_is_dir && !dest_is_dir) { - std::u8string message = u8"Cannot overwrite non-directory " + - src_path.u8string() + u8" with directory " + - dest_path.u8string(); + std::string message = + "Cannot overwrite non-directory %s with directory %s"; return THROW_ERR_FS_CP_DIR_TO_NON_DIR( - env, reinterpret_cast(message.c_str())); + env, message.c_str(), src_path_str, dest_path_str); } if (!src_is_dir && dest_is_dir) { - std::u8string message = u8"Cannot overwrite directory " + - dest_path.u8string() + u8" with non-directory " + - src_path.u8string(); + std::string message = + "Cannot overwrite directory %s with non-directory %s"; return THROW_ERR_FS_CP_NON_DIR_TO_DIR( - env, reinterpret_cast(message.c_str())); + env, message.c_str(), dest_path_str, src_path_str); } } - std::u8string dest_path_str = dest_path.u8string(); - std::u8string src_path_str = src_path.u8string(); if (!src_path_str.ends_with(std::filesystem::path::preferred_separator)) { src_path_str += std::filesystem::path::preferred_separator; } // Check if dest_path is a subdirectory of src_path. if (src_is_dir && dest_path_str.starts_with(src_path_str)) { - std::u8string message = u8"Cannot copy " + src_path.u8string() + - u8" to a subdirectory of self " + - dest_path.u8string(); + std::string message = "Cannot copy %s to a subdirectory of self %s"; return THROW_ERR_FS_CP_EINVAL( - env, reinterpret_cast(message.c_str())); + env, message.c_str(), src_path_str, dest_path_str); } auto dest_parent = dest_path.parent_path(); @@ -3227,11 +3270,9 @@ 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::u8string message = u8"Cannot copy " + src_path.u8string() + - u8" to a subdirectory of self " + - dest_path.u8string(); + std::string message = "Cannot copy %s to a subdirectory of self %s"; return THROW_ERR_FS_CP_EINVAL( - env, reinterpret_cast(message.c_str())); + env, message.c_str(), src_path_str, dest_path_str); } // If equivalent fails, it's highly likely that dest_parent does not exist @@ -3243,29 +3284,23 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { } if (src_is_dir && !recursive) { - 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())); + std::string message = + "Recursive option not enabled, cannot copy a directory: %s"; + return THROW_ERR_FS_EISDIR(env, message.c_str(), src_path_str); } switch (src_status.type()) { case std::filesystem::file_type::socket: { - std::u8string message = u8"Cannot copy a socket file: " + dest_path_str; - return THROW_ERR_FS_CP_SOCKET( - env, reinterpret_cast(message.c_str())); + std::string message = "Cannot copy a socket file: %s"; + return THROW_ERR_FS_CP_SOCKET(env, message.c_str(), dest_path_str); } case std::filesystem::file_type::fifo: { - 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())); + std::string message = "Cannot copy a FIFO pipe: %s"; + return THROW_ERR_FS_CP_FIFO_PIPE(env, message.c_str(), dest_path_str); } case std::filesystem::file_type::unknown: { - 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())); + std::string message = "Cannot copy an unknown file type: %s"; + return THROW_ERR_FS_CP_UNKNOWN(env, message.c_str(), dest_path_str); } default: break; diff --git a/test/parallel/test-fs-cp.mjs b/test/parallel/test-fs-cp.mjs index d34160aa5ac..0f3274ada62 100644 --- a/test/parallel/test-fs-cp.mjs +++ b/test/parallel/test-fs-cp.mjs @@ -25,7 +25,7 @@ tmpdir.refresh(); let dirc = 0; function nextdir(dirname) { - return tmpdir.resolve(dirname || `copy_${++dirc}`); + return tmpdir.resolve(dirname || `copy_%${++dirc}`); } // Synchronous implementation of copy.