From ec1fc03034f15612f2d0d868f64c039fc9f79192 Mon Sep 17 00:00:00 2001 From: Fedor Indutny <238531+indutny@users.noreply.github.com> Date: Tue, 4 Nov 2025 11:49:31 -0800 Subject: [PATCH 1/5] src: use CP_UTF8 for wide file names on win32 `src/node_modules.cc` needs to be consistent with `src/node_file.cc` in how it translates the utf8 strings to `std::wstring` otherwise we might end up in situation where we can read the source code of imported package from disk, but fail to recognize that it is an ESM (or CJS) and cause runtime errors. This type of error is possible on Windows when the path contains unicode characters and "Language for non-Unicode programs" is set to "Chinese (Traditional, Taiwan)". See: #58768 --- src/node_file.cc | 23 ----------------------- src/node_modules.cc | 33 +++++++++++++++++++++++---------- src/util-inl.h | 23 +++++++++++++++++++++++ src/util.h | 1 + 4 files changed, 47 insertions(+), 33 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index f0d512069c6dc6..bbae25a2892cd5 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3173,29 +3173,6 @@ static void GetFormatOfExtensionlessFile( #define BufferValueToPath(str) \ std::filesystem::path(ConvertToWideString(str.ToString(), CP_UTF8)) -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 diff --git a/src/node_modules.cc b/src/node_modules.cc index 7e615a691a5a14..550bfbf7f27dd1 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc @@ -296,22 +296,35 @@ const BindingData::PackageConfig* BindingData::TraverseParent( // Stop the search when the process doesn't have permissions // to walk upwards - if (is_permissions_enabled && - !env->permission()->is_granted( - env, - permission::PermissionScope::kFileSystemRead, - current_path.generic_string())) [[unlikely]] { - return nullptr; + if (is_permissions_enabled) { + std::string generic_path; +#ifdef _WIN32 + generic_path = ConvertWideToUTF8(current_path.generic_wstring()); +#else // _WIN32 + generic_path = current_path.generic_string(); +#endif // _WIN32 + // + if (!env->permission()->is_granted( + env, permission::PermissionScope::kFileSystemRead, generic_path)) + [[unlikely]] { + return nullptr; + } } // Check if the path ends with `/node_modules` - if (current_path.generic_string().ends_with("/node_modules")) { + if (current_path.filename() == "node_modules") { return nullptr; } auto package_json_path = current_path / "package.json"; - auto package_json = - GetPackageJSON(realm, package_json_path.string(), nullptr); + + std::string package_json_path_str; +#ifdef _WIN32 + package_json_path_str = ConvertWideToUTF8(package_json_path.wstring()); +#else // _WIN32 + package_json_path_str = package_json_path.string(); +#endif // _WIN32 + auto package_json = GetPackageJSON(realm, package_json_path_str, nullptr); if (package_json != nullptr) { return package_json; } @@ -341,7 +354,7 @@ void BindingData::GetNearestParentPackageJSONType( std::filesystem::path path; #ifdef _WIN32 - std::wstring wide_path = ConvertToWideString(path_value_str, GetACP()); + std::wstring wide_path = ConvertToWideString(path_value_str, CP_UTF8); path = std::filesystem::path(wide_path); #else path = std::filesystem::path(path_value_str); diff --git a/src/util-inl.h b/src/util-inl.h index d07bceb425f008..2e470bf0f27495 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -731,6 +731,29 @@ inline std::wstring ConvertToWideString(const std::string& str, size_needed); return wstrTo; } + +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; +} #endif // _WIN32 inline v8::MaybeLocal NewDictionaryInstance( diff --git a/src/util.h b/src/util.h index 6dfccf3a342da3..80ec6e01ebbd7b 100644 --- a/src/util.h +++ b/src/util.h @@ -1039,6 +1039,7 @@ class JSONOutputStream final : public v8::OutputStream { // case insensitive. inline bool IsWindowsBatchFile(const char* filename); inline std::wstring ConvertToWideString(const std::string& str, UINT code_page); +inline std::string ConvertWideToUTF8(const std::wstring& wstr); #endif // _WIN32 // A helper to create a new instance of the dictionary template. From db29a1da2a8dac027600469fd39c58686ea679c6 Mon Sep 17 00:00:00 2001 From: Fedor Indutny <238531+indutny@users.noreply.github.com> Date: Wed, 5 Nov 2025 09:28:31 -0800 Subject: [PATCH 2/5] address feedback --- src/node_file.cc | 29 ++++++++--------------------- src/node_modules.cc | 35 ++++++++--------------------------- src/util-inl.h | 37 +++++++++++++++++++++++++++++++++---- src/util.h | 10 ++++++++-- 4 files changed, 57 insertions(+), 54 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index bbae25a2892cd5..d8a2791a086354 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -3169,19 +3169,6 @@ static void GetFormatOfExtensionlessFile( return args.GetReturnValue().Set(EXTENSIONLESS_FORMAT_JAVASCRIPT); } -#ifdef _WIN32 -#define BufferValueToPath(str) \ - std::filesystem::path(ConvertToWideString(str.ToString(), CP_UTF8)) - -#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(); @@ -3194,7 +3181,7 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { THROW_IF_INSUFFICIENT_PERMISSIONS( env, permission::PermissionScope::kFileSystemRead, src.ToStringView()); - auto src_path = BufferValueToPath(src); + auto src_path = src.ToPath(); BufferValue dest(isolate, args[1]); CHECK_NOT_NULL(*dest); @@ -3202,7 +3189,7 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo& args) { THROW_IF_INSUFFICIENT_PERMISSIONS( env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView()); - auto dest_path = BufferValueToPath(dest); + auto dest_path = dest.ToPath(); bool dereference = args[2]->IsTrue(); bool recursive = args[3]->IsTrue(); @@ -3231,8 +3218,8 @@ 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); + auto src_path_str = ConvertPathToUTF8(src_path); + auto dest_path_str = ConvertPathToUTF8(dest_path); if (!error_code) { // Check if src and dest are identical. @@ -3327,7 +3314,7 @@ static bool CopyUtimes(const std::filesystem::path& src, uv_fs_t req; auto cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); }); - auto src_path_str = PathToString(src); + auto src_path_str = ConvertPathToUTF8(src); int result = uv_fs_stat(nullptr, &req, src_path_str.c_str(), nullptr); if (is_uv_error(result)) { env->ThrowUVException(result, "stat", nullptr, src_path_str.c_str()); @@ -3338,7 +3325,7 @@ static bool CopyUtimes(const std::filesystem::path& src, const double source_atime = s->st_atim.tv_sec + s->st_atim.tv_nsec / 1e9; const double source_mtime = s->st_mtim.tv_sec + s->st_mtim.tv_nsec / 1e9; - auto dest_file_path_str = PathToString(dest); + auto dest_file_path_str = ConvertPathToUTF8(dest); int utime_result = uv_fs_utime(nullptr, &req, dest_file_path_str.c_str(), @@ -3473,7 +3460,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { std::error_code error; for (auto dir_entry : std::filesystem::directory_iterator(src)) { auto dest_file_path = dest / dir_entry.path().filename(); - auto dest_str = PathToString(dest); + auto dest_str = ConvertPathToUTF8(dest); if (dir_entry.is_symlink()) { if (verbatim_symlinks) { @@ -3536,7 +3523,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo& args) { } } else if (std::filesystem::is_regular_file(dest_file_path)) { if (!dereference || (!force && error_on_exist)) { - auto dest_file_path_str = PathToString(dest_file_path); + auto dest_file_path_str = ConvertPathToUTF8(dest_file_path); env->ThrowStdErrException( std::make_error_code(std::errc::file_exists), "cp", diff --git a/src/node_modules.cc b/src/node_modules.cc index 550bfbf7f27dd1..d59426109e647c 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc @@ -297,16 +297,10 @@ const BindingData::PackageConfig* BindingData::TraverseParent( // Stop the search when the process doesn't have permissions // to walk upwards if (is_permissions_enabled) { - std::string generic_path; -#ifdef _WIN32 - generic_path = ConvertWideToUTF8(current_path.generic_wstring()); -#else // _WIN32 - generic_path = current_path.generic_string(); -#endif // _WIN32 - // if (!env->permission()->is_granted( - env, permission::PermissionScope::kFileSystemRead, generic_path)) - [[unlikely]] { + env, + permission::PermissionScope::kFileSystemRead, + ConvertGenericPathToUTF8(current_path))) [[unlikely]] { return nullptr; } } @@ -318,13 +312,8 @@ const BindingData::PackageConfig* BindingData::TraverseParent( auto package_json_path = current_path / "package.json"; - std::string package_json_path_str; -#ifdef _WIN32 - package_json_path_str = ConvertWideToUTF8(package_json_path.wstring()); -#else // _WIN32 - package_json_path_str = package_json_path.string(); -#endif // _WIN32 - auto package_json = GetPackageJSON(realm, package_json_path_str, nullptr); + auto package_json = + GetPackageJSON(realm, ConvertPathToUTF8(package_json_path), nullptr); if (package_json != nullptr) { return package_json; } @@ -346,20 +335,12 @@ void BindingData::GetNearestParentPackageJSONType( ToNamespacedPath(realm->env(), &path_value); - std::string path_value_str = path_value.ToString(); + auto path = path_value.ToPath(); + if (slashCheck) { - path_value_str.push_back(kPathSeparator); + path /= ""; } - std::filesystem::path path; - -#ifdef _WIN32 - std::wstring wide_path = ConvertToWideString(path_value_str, CP_UTF8); - path = std::filesystem::path(wide_path); -#else - path = std::filesystem::path(path_value_str); -#endif - auto package_json = TraverseParent(realm, path); if (package_json == nullptr) { diff --git a/src/util-inl.h b/src/util-inl.h index 2e470bf0f27495..d23098c98aa7b1 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -718,12 +718,11 @@ inline bool IsWindowsBatchFile(const char* filename) { return !extension.empty() && (extension == "cmd" || extension == "bat"); } -inline std::wstring ConvertToWideString(const std::string& str, - UINT code_page) { +inline std::wstring ConvertUTF8ToWideString(const std::string& str) { int size_needed = MultiByteToWideChar( - code_page, 0, &str[0], static_cast(str.size()), nullptr, 0); + CP_UTF8, 0, &str[0], static_cast(str.size()), nullptr, 0); std::wstring wstrTo(size_needed, 0); - MultiByteToWideChar(code_page, + MultiByteToWideChar(CP_UTF8, 0, &str[0], static_cast(str.size()), @@ -754,6 +753,36 @@ std::string ConvertWideToUTF8(const std::wstring& wstr) { nullptr); return strTo; } + +template +std::filesystem::path MaybeStackBuffer::ToPath() const { + std::wstring wide_path = ConvertUTF8ToWideString(ToString()); + return std::filesystem::path(wide_path); +} + +std::string ConvertPathToUTF8(const std::filesystem::path& path) { + return ConvertWideStringToUTF8(path.wstring()); +} + +std::string ConvertGenericPathToUTF8(const std::filesystem::path& path) { + return ConvertWideStringToUTF8(path.generic_wstring()); +} + +#else // _WIN32 + +template +std::filesystem::path MaybeStackBuffer::ToPath() const { + return std::filesystem::path(ToStringView()); +} + +std::string ConvertPathToUTF8(const std::filesystem::path& path) { + return path.native(); +} + +std::string ConvertGenericPathToUTF8(const std::filesystem::path& path) { + return path.generic_string(); +} + #endif // _WIN32 inline v8::MaybeLocal NewDictionaryInstance( diff --git a/src/util.h b/src/util.h index 80ec6e01ebbd7b..7a594290a4dc70 100644 --- a/src/util.h +++ b/src/util.h @@ -507,6 +507,7 @@ class MaybeStackBuffer { inline std::basic_string_view ToStringView() const { return {out(), length()}; } + inline std::filesystem::path ToPath() const; private: size_t length_; @@ -1038,10 +1039,15 @@ class JSONOutputStream final : public v8::OutputStream { // Returns true if OS==Windows and filename ends in .bat or .cmd, // case insensitive. inline bool IsWindowsBatchFile(const char* filename); -inline std::wstring ConvertToWideString(const std::string& str, UINT code_page); -inline std::string ConvertWideToUTF8(const std::wstring& wstr); +inline std::wstring ConvertUTF8ToWideString(const std::string& str); +inline std::string ConvertWideStringToUTF8(const std::wstring& wstr); + #endif // _WIN32 +inline std::filesystem::path ConvertUTF8ToPath(const std::string& str); +inline std::string ConvertPathToUTF8(const std::filesystem::path& path); +inline std::string ConvertGenericPathToUTF8(const std::filesystem::path& path); + // A helper to create a new instance of the dictionary template. // Unlike v8::DictionaryTemplate::NewInstance, this method will // check that all properties have been set (are not empty MaybeLocals) From 8d686c3c804c813cbde0ccfa52197d9e53abba59 Mon Sep 17 00:00:00 2001 From: Fedor Indutny <238531+indutny@users.noreply.github.com> Date: Wed, 5 Nov 2025 09:31:09 -0800 Subject: [PATCH 3/5] fix windows --- src/util-inl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util-inl.h b/src/util-inl.h index d23098c98aa7b1..6898e8ea794675 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -731,7 +731,7 @@ inline std::wstring ConvertUTF8ToWideString(const std::string& str) { return wstrTo; } -std::string ConvertWideToUTF8(const std::wstring& wstr) { +std::string ConvertWideStringToUTF8(const std::wstring& wstr) { if (wstr.empty()) return std::string(); int size_needed = WideCharToMultiByte(CP_UTF8, From f620bb4f81b5e4bb56983b7eb8a4bc4bb4debda5 Mon Sep 17 00:00:00 2001 From: Fedor Indutny <238531+indutny@users.noreply.github.com> Date: Wed, 5 Nov 2025 10:55:18 -0800 Subject: [PATCH 4/5] fix one more windows error --- src/compile_cache.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/compile_cache.cc b/src/compile_cache.cc index 5e2c56cb925cc8..0920747012f072 100644 --- a/src/compile_cache.cc +++ b/src/compile_cache.cc @@ -232,10 +232,8 @@ static std::string GetRelativePath(std::string_view path, // the paths to wide strings before using std::filesystem::path. // On other platforms, std::filesystem::path can handle UTF-8 directly. #ifdef _WIN32 - std::filesystem::path module_path( - ConvertToWideString(std::string(path), CP_UTF8)); - std::filesystem::path base_path( - ConvertToWideString(std::string(base), CP_UTF8)); + std::filesystem::path module_path(ConvertUTF8ToWideString(std::string(path))); + std::filesystem::path base_path(ConvertUTF8ToWideString(std::string(base))); #else std::filesystem::path module_path(path); std::filesystem::path base_path(base); From 297b2b92b8359f18d96905c82a0718f39c3873ad Mon Sep 17 00:00:00 2001 From: Fedor Indutny <238531+indutny@users.noreply.github.com> Date: Wed, 5 Nov 2025 15:12:24 -0800 Subject: [PATCH 5/5] Update src/util.h Co-authored-by: Joyee Cheung --- src/util.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util.h b/src/util.h index 7a594290a4dc70..aa2a47ce0f1ca9 100644 --- a/src/util.h +++ b/src/util.h @@ -507,6 +507,7 @@ class MaybeStackBuffer { inline std::basic_string_view ToStringView() const { return {out(), length()}; } + // This can only be used if the buffer contains path data in UTF8 inline std::filesystem::path ToPath() const; private: