Skip to content

Commit 0ebf839

Browse files
indutnyaduh95
indutny
authored and
aduh95
committed
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 PR-URL: #60575 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
1 parent a31ad37 commit 0ebf839

File tree

5 files changed

+88
-72
lines changed

5 files changed

+88
-72
lines changed

‎src/compile_cache.cc‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,8 @@ static std::string GetRelativePath(std::string_view path,
232232
// the paths to wide strings before using std::filesystem::path.
233233
// On other platforms, std::filesystem::path can handle UTF-8 directly.
234234
#ifdef _WIN32
235-
std::filesystem::path module_path(
236-
ConvertToWideString(std::string(path), CP_UTF8));
237-
std::filesystem::path base_path(
238-
ConvertToWideString(std::string(base), CP_UTF8));
235+
std::filesystem::path module_path(ConvertUTF8ToWideString(std::string(path)));
236+
std::filesystem::path base_path(ConvertUTF8ToWideString(std::string(base)));
239237
#else
240238
std::filesystem::path module_path(path);
241239
std::filesystem::path base_path(base);

‎src/node_file.cc‎

Lines changed: 8 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -3184,42 +3184,6 @@ static void GetFormatOfExtensionlessFile(
31843184
return args.GetReturnValue().Set(EXTENSIONLESS_FORMAT_JAVASCRIPT);
31853185
}
31863186

3187-
#ifdef _WIN32
3188-
#define BufferValueToPath(str) \
3189-
std::filesystem::path(ConvertToWideString(str.ToString(), CP_UTF8))
3190-
3191-
std::string ConvertWideToUTF8(const std::wstring& wstr) {
3192-
if (wstr.empty()) return std::string();
3193-
3194-
int size_needed = WideCharToMultiByte(CP_UTF8,
3195-
0,
3196-
&wstr[0],
3197-
static_cast<int>(wstr.size()),
3198-
nullptr,
3199-
0,
3200-
nullptr,
3201-
nullptr);
3202-
std::string strTo(size_needed, 0);
3203-
WideCharToMultiByte(CP_UTF8,
3204-
0,
3205-
&wstr[0],
3206-
static_cast<int>(wstr.size()),
3207-
&strTo[0],
3208-
size_needed,
3209-
nullptr,
3210-
nullptr);
3211-
return strTo;
3212-
}
3213-
3214-
#define PathToString(path) ConvertWideToUTF8(path.wstring());
3215-
3216-
#else // _WIN32
3217-
3218-
#define BufferValueToPath(str) std::filesystem::path(str.ToStringView());
3219-
#define PathToString(path) path.native();
3220-
3221-
#endif // _WIN32
3222-
32233187
static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
32243188
Environment* env = Environment::GetCurrent(args);
32253189
Isolate* isolate = env->isolate();
@@ -3232,15 +3196,15 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
32323196
THROW_IF_INSUFFICIENT_PERMISSIONS(
32333197
env, permission::PermissionScope::kFileSystemRead, src.ToStringView());
32343198

3235-
auto src_path = BufferValueToPath(src);
3199+
auto src_path = src.ToPath();
32363200

32373201
BufferValue dest(isolate, args[1]);
32383202
CHECK_NOT_NULL(*dest);
32393203
ToNamespacedPath(env, &dest);
32403204
THROW_IF_INSUFFICIENT_PERMISSIONS(
32413205
env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView());
32423206

3243-
auto dest_path = BufferValueToPath(dest);
3207+
auto dest_path = dest.ToPath();
32443208
bool dereference = args[2]->IsTrue();
32453209
bool recursive = args[3]->IsTrue();
32463210

@@ -3269,8 +3233,8 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
32693233
(src_status.type() == std::filesystem::file_type::directory) ||
32703234
(dereference && src_status.type() == std::filesystem::file_type::symlink);
32713235

3272-
auto src_path_str = PathToString(src_path);
3273-
auto dest_path_str = PathToString(dest_path);
3236+
auto src_path_str = ConvertPathToUTF8(src_path);
3237+
auto dest_path_str = ConvertPathToUTF8(dest_path);
32743238

32753239
if (!error_code) {
32763240
// Check if src and dest are identical.
@@ -3365,7 +3329,7 @@ static bool CopyUtimes(const std::filesystem::path& src,
33653329
uv_fs_t req;
33663330
auto cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
33673331

3368-
auto src_path_str = PathToString(src);
3332+
auto src_path_str = ConvertPathToUTF8(src);
33693333
int result = uv_fs_stat(nullptr, &req, src_path_str.c_str(), nullptr);
33703334
if (is_uv_error(result)) {
33713335
env->ThrowUVException(result, "stat", nullptr, src_path_str.c_str());
@@ -3376,7 +3340,7 @@ static bool CopyUtimes(const std::filesystem::path& src,
33763340
const double source_atime = s->st_atim.tv_sec + s->st_atim.tv_nsec / 1e9;
33773341
const double source_mtime = s->st_mtim.tv_sec + s->st_mtim.tv_nsec / 1e9;
33783342

3379-
auto dest_file_path_str = PathToString(dest);
3343+
auto dest_file_path_str = ConvertPathToUTF8(dest);
33803344
int utime_result = uv_fs_utime(nullptr,
33813345
&req,
33823346
dest_file_path_str.c_str(),
@@ -3511,7 +3475,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo<Value>& args) {
35113475
std::error_code error;
35123476
for (auto dir_entry : std::filesystem::directory_iterator(src)) {
35133477
auto dest_file_path = dest / dir_entry.path().filename();
3514-
auto dest_str = PathToString(dest);
3478+
auto dest_str = ConvertPathToUTF8(dest);
35153479

35163480
if (dir_entry.is_symlink()) {
35173481
if (verbatim_symlinks) {
@@ -3570,7 +3534,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo<Value>& args) {
35703534
}
35713535
} else if (std::filesystem::is_regular_file(dest_file_path)) {
35723536
if (!dereference || (!force && error_on_exist)) {
3573-
auto dest_file_path_str = PathToString(dest_file_path);
3537+
auto dest_file_path_str = ConvertPathToUTF8(dest_file_path);
35743538
env->ThrowStdErrException(
35753539
std::make_error_code(std::errc::file_exists),
35763540
"cp",

‎src/node_modules.cc‎

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -296,22 +296,24 @@ const BindingData::PackageConfig* BindingData::TraverseParent(
296296

297297
// Stop the search when the process doesn't have permissions
298298
// to walk upwards
299-
if (is_permissions_enabled &&
300-
!env->permission()->is_granted(
301-
env,
302-
permission::PermissionScope::kFileSystemRead,
303-
current_path.generic_string())) [[unlikely]] {
304-
return nullptr;
299+
if (is_permissions_enabled) {
300+
if (!env->permission()->is_granted(
301+
env,
302+
permission::PermissionScope::kFileSystemRead,
303+
ConvertGenericPathToUTF8(current_path))) [[unlikely]] {
304+
return nullptr;
305+
}
305306
}
306307

307308
// Check if the path ends with `/node_modules`
308-
if (current_path.generic_string().ends_with("/node_modules")) {
309+
if (current_path.filename() == "node_modules") {
309310
return nullptr;
310311
}
311312

312313
auto package_json_path = current_path / "package.json";
314+
313315
auto package_json =
314-
GetPackageJSON(realm, package_json_path.string(), nullptr);
316+
GetPackageJSON(realm, ConvertPathToUTF8(package_json_path), nullptr);
315317
if (package_json != nullptr) {
316318
return package_json;
317319
}
@@ -333,20 +335,12 @@ void BindingData::GetNearestParentPackageJSONType(
333335

334336
ToNamespacedPath(realm->env(), &path_value);
335337

336-
std::string path_value_str = path_value.ToString();
338+
auto path = path_value.ToPath();
339+
337340
if (slashCheck) {
338-
path_value_str.push_back(kPathSeparator);
341+
path /= "";
339342
}
340343

341-
std::filesystem::path path;
342-
343-
#ifdef _WIN32
344-
std::wstring wide_path = ConvertToWideString(path_value_str, GetACP());
345-
path = std::filesystem::path(wide_path);
346-
#else
347-
path = std::filesystem::path(path_value_str);
348-
#endif
349-
350344
auto package_json = TraverseParent(realm, path);
351345

352346
if (package_json == nullptr) {

‎src/util-inl.h‎

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -718,19 +718,71 @@ inline bool IsWindowsBatchFile(const char* filename) {
718718
return !extension.empty() && (extension == "cmd" || extension == "bat");
719719
}
720720

721-
inline std::wstring ConvertToWideString(const std::string& str,
722-
UINT code_page) {
721+
inline std::wstring ConvertUTF8ToWideString(const std::string& str) {
723722
int size_needed = MultiByteToWideChar(
724-
code_page, 0, &str[0], static_cast<int>(str.size()), nullptr, 0);
723+
CP_UTF8, 0, &str[0], static_cast<int>(str.size()), nullptr, 0);
725724
std::wstring wstrTo(size_needed, 0);
726-
MultiByteToWideChar(code_page,
725+
MultiByteToWideChar(CP_UTF8,
727726
0,
728727
&str[0],
729728
static_cast<int>(str.size()),
730729
&wstrTo[0],
731730
size_needed);
732731
return wstrTo;
733732
}
733+
734+
std::string ConvertWideStringToUTF8(const std::wstring& wstr) {
735+
if (wstr.empty()) return std::string();
736+
737+
int size_needed = WideCharToMultiByte(CP_UTF8,
738+
0,
739+
&wstr[0],
740+
static_cast<int>(wstr.size()),
741+
nullptr,
742+
0,
743+
nullptr,
744+
nullptr);
745+
std::string strTo(size_needed, 0);
746+
WideCharToMultiByte(CP_UTF8,
747+
0,
748+
&wstr[0],
749+
static_cast<int>(wstr.size()),
750+
&strTo[0],
751+
size_needed,
752+
nullptr,
753+
nullptr);
754+
return strTo;
755+
}
756+
757+
template <typename T, size_t kStackStorageSize>
758+
std::filesystem::path MaybeStackBuffer<T, kStackStorageSize>::ToPath() const {
759+
std::wstring wide_path = ConvertUTF8ToWideString(ToString());
760+
return std::filesystem::path(wide_path);
761+
}
762+
763+
std::string ConvertPathToUTF8(const std::filesystem::path& path) {
764+
return ConvertWideStringToUTF8(path.wstring());
765+
}
766+
767+
std::string ConvertGenericPathToUTF8(const std::filesystem::path& path) {
768+
return ConvertWideStringToUTF8(path.generic_wstring());
769+
}
770+
771+
#else // _WIN32
772+
773+
template <typename T, size_t kStackStorageSize>
774+
std::filesystem::path MaybeStackBuffer<T, kStackStorageSize>::ToPath() const {
775+
return std::filesystem::path(ToStringView());
776+
}
777+
778+
std::string ConvertPathToUTF8(const std::filesystem::path& path) {
779+
return path.native();
780+
}
781+
782+
std::string ConvertGenericPathToUTF8(const std::filesystem::path& path) {
783+
return path.generic_string();
784+
}
785+
734786
#endif // _WIN32
735787

736788
inline v8::MaybeLocal<v8::Object> NewDictionaryInstance(

‎src/util.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,8 @@ class MaybeStackBuffer {
507507
inline std::basic_string_view<T> ToStringView() const {
508508
return {out(), length()};
509509
}
510+
// This can only be used if the buffer contains path data in UTF8
511+
inline std::filesystem::path ToPath() const;
510512

511513
private:
512514
size_t length_;
@@ -1038,9 +1040,15 @@ class JSONOutputStream final : public v8::OutputStream {
10381040
// Returns true if OS==Windows and filename ends in .bat or .cmd,
10391041
// case insensitive.
10401042
inline bool IsWindowsBatchFile(const char* filename);
1041-
inline std::wstring ConvertToWideString(const std::string& str, UINT code_page);
1043+
inline std::wstring ConvertUTF8ToWideString(const std::string& str);
1044+
inline std::string ConvertWideStringToUTF8(const std::wstring& wstr);
1045+
10421046
#endif // _WIN32
10431047

1048+
inline std::filesystem::path ConvertUTF8ToPath(const std::string& str);
1049+
inline std::string ConvertPathToUTF8(const std::filesystem::path& path);
1050+
inline std::string ConvertGenericPathToUTF8(const std::filesystem::path& path);
1051+
10441052
// A helper to create a new instance of the dictionary template.
10451053
// Unlike v8::DictionaryTemplate::NewInstance, this method will
10461054
// check that all properties have been set (are not empty MaybeLocals)

0 commit comments

Comments
 (0)