Skip to content

Commit d92ec21

Browse files
indutnytargos
indutny
authored and
targos
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 92ea669 commit d92ec21

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
@@ -3175,42 +3175,6 @@ static void GetFormatOfExtensionlessFile(
31753175
return args.GetReturnValue().Set(EXTENSIONLESS_FORMAT_JAVASCRIPT);
31763176
}
31773177

3178-
#ifdef _WIN32
3179-
#define BufferValueToPath(str) \
3180-
std::filesystem::path(ConvertToWideString(str.ToString(), CP_UTF8))
3181-
3182-
std::string ConvertWideToUTF8(const std::wstring& wstr) {
3183-
if (wstr.empty()) return std::string();
3184-
3185-
int size_needed = WideCharToMultiByte(CP_UTF8,
3186-
0,
3187-
&wstr[0],
3188-
static_cast<int>(wstr.size()),
3189-
nullptr,
3190-
0,
3191-
nullptr,
3192-
nullptr);
3193-
std::string strTo(size_needed, 0);
3194-
WideCharToMultiByte(CP_UTF8,
3195-
0,
3196-
&wstr[0],
3197-
static_cast<int>(wstr.size()),
3198-
&strTo[0],
3199-
size_needed,
3200-
nullptr,
3201-
nullptr);
3202-
return strTo;
3203-
}
3204-
3205-
#define PathToString(path) ConvertWideToUTF8(path.wstring());
3206-
3207-
#else // _WIN32
3208-
3209-
#define BufferValueToPath(str) std::filesystem::path(str.ToStringView());
3210-
#define PathToString(path) path.native();
3211-
3212-
#endif // _WIN32
3213-
32143178
static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
32153179
Environment* env = Environment::GetCurrent(args);
32163180
Isolate* isolate = env->isolate();
@@ -3223,15 +3187,15 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
32233187
THROW_IF_INSUFFICIENT_PERMISSIONS(
32243188
env, permission::PermissionScope::kFileSystemRead, src.ToStringView());
32253189

3226-
auto src_path = BufferValueToPath(src);
3190+
auto src_path = src.ToPath();
32273191

32283192
BufferValue dest(isolate, args[1]);
32293193
CHECK_NOT_NULL(*dest);
32303194
ToNamespacedPath(env, &dest);
32313195
THROW_IF_INSUFFICIENT_PERMISSIONS(
32323196
env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView());
32333197

3234-
auto dest_path = BufferValueToPath(dest);
3198+
auto dest_path = dest.ToPath();
32353199
bool dereference = args[2]->IsTrue();
32363200
bool recursive = args[3]->IsTrue();
32373201

@@ -3260,8 +3224,8 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
32603224
(src_status.type() == std::filesystem::file_type::directory) ||
32613225
(dereference && src_status.type() == std::filesystem::file_type::symlink);
32623226

3263-
auto src_path_str = PathToString(src_path);
3264-
auto dest_path_str = PathToString(dest_path);
3227+
auto src_path_str = ConvertPathToUTF8(src_path);
3228+
auto dest_path_str = ConvertPathToUTF8(dest_path);
32653229

32663230
if (!error_code) {
32673231
// Check if src and dest are identical.
@@ -3356,7 +3320,7 @@ static bool CopyUtimes(const std::filesystem::path& src,
33563320
uv_fs_t req;
33573321
auto cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
33583322

3359-
auto src_path_str = PathToString(src);
3323+
auto src_path_str = ConvertPathToUTF8(src);
33603324
int result = uv_fs_stat(nullptr, &req, src_path_str.c_str(), nullptr);
33613325
if (is_uv_error(result)) {
33623326
env->ThrowUVException(result, "stat", nullptr, src_path_str.c_str());
@@ -3367,7 +3331,7 @@ static bool CopyUtimes(const std::filesystem::path& src,
33673331
const double source_atime = s->st_atim.tv_sec + s->st_atim.tv_nsec / 1e9;
33683332
const double source_mtime = s->st_mtim.tv_sec + s->st_mtim.tv_nsec / 1e9;
33693333

3370-
auto dest_file_path_str = PathToString(dest);
3334+
auto dest_file_path_str = ConvertPathToUTF8(dest);
33713335
int utime_result = uv_fs_utime(nullptr,
33723336
&req,
33733337
dest_file_path_str.c_str(),
@@ -3502,7 +3466,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo<Value>& args) {
35023466
std::error_code error;
35033467
for (auto dir_entry : std::filesystem::directory_iterator(src)) {
35043468
auto dest_file_path = dest / dir_entry.path().filename();
3505-
auto dest_str = PathToString(dest);
3469+
auto dest_str = ConvertPathToUTF8(dest);
35063470

35073471
if (dir_entry.is_symlink()) {
35083472
if (verbatim_symlinks) {
@@ -3561,7 +3525,7 @@ static void CpSyncCopyDir(const FunctionCallbackInfo<Value>& args) {
35613525
}
35623526
} else if (std::filesystem::is_regular_file(dest_file_path)) {
35633527
if (!dereference || (!force && error_on_exist)) {
3564-
auto dest_file_path_str = PathToString(dest_file_path);
3528+
auto dest_file_path_str = ConvertPathToUTF8(dest_file_path);
35653529
env->ThrowStdErrException(
35663530
std::make_error_code(std::errc::file_exists),
35673531
"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)