nodejs · islandryu · Nov 15, 2024 · Nov 15, 2024 · Nov 15, 2024 · Nov 15, 2024
diff --git a/src/node_file.cc b/src/node_file.cc
@@ -3125,55 +3125,6 @@ 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<int>(str.size()), nullptr, 0);
-  std::wstring wstrTo(size_needed, 0);
-  MultiByteToWideChar(CP_UTF8,
-                      0,
-                      &str[0],
-                      static_cast<int>(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<int>(wstr.size()),
-                                        nullptr,
-                                        0,
-                                        nullptr,
-                                        nullptr);
-  std::string strTo(size_needed, 0);
-  WideCharToMultiByte(CP_UTF8,
-                      0,
-                      &wstr[0],
-                      static_cast<int>(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<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();
@@ -3186,15 +3137,15 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
   THROW_IF_INSUFFICIENT_PERMISSIONS(
       env, permission::PermissionScope::kFileSystemRead, src.ToStringView());
 
-  auto src_path = BufferValueToPath(src);
+  auto src_path = StringViewToPath(src.ToStringView());
 
   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 = BufferValueToPath(dest);
+  auto dest_path = StringViewToPath(dest.ToStringView());
   bool dereference = args[2]->IsTrue();
   bool recursive = args[3]->IsTrue();
 

diff --git a/src/node_modules.cc b/src/node_modules.cc
@@ -337,14 +337,12 @@ void BindingData::GetNearestParentPackageJSON(
   bool slashCheck = path_value.ToStringView().ends_with(kPathSeparator);
 
   ToNamespacedPath(realm->env(), &path_value);
-
-  std::string path_value_str = path_value.ToString();
+  std::filesystem::path path = StringViewToPath(path_value.ToStringView());
   if (slashCheck) {
-    path_value_str.push_back(kPathSeparator);
+    path += kPathSeparator;
   }
 
-  auto package_json =
-      TraverseParent(realm, std::filesystem::path(path_value_str));
+  auto package_json = TraverseParent(realm, path);
 
   if (package_json != nullptr) {
     args.GetReturnValue().Set(package_json->Serialize(realm));
@@ -363,14 +361,12 @@ void BindingData::GetNearestParentPackageJSONType(
   bool slashCheck = path_value.ToStringView().ends_with(kPathSeparator);
 
   ToNamespacedPath(realm->env(), &path_value);
-
-  std::string path_value_str = path_value.ToString();
+  std::filesystem::path path = StringViewToPath(path_value.ToStringView());
   if (slashCheck) {
-    path_value_str.push_back(kPathSeparator);
+    path += kPathSeparator;
   }
 
-  auto package_json =
-      TraverseParent(realm, std::filesystem::path(path_value_str));
+  auto package_json = TraverseParent(realm, path);
 
   if (package_json == nullptr) {
     return;

diff --git a/src/util.cc b/src/util.cc
@@ -885,4 +885,42 @@ v8::Maybe<int> GetValidFileMode(Environment* env,
   return v8::Just(mode);
 }
 
+#ifdef _WIN32
+std::wstring ConvertToWideString(std::string_view strView) {
+  int size_needed = MultiByteToWideChar(
+      CP_UTF8, 0, strView.data(), static_cast<int>(strView.size()), nullptr, 0);
+  std::wstring wstrTo(size_needed, 0);
+  MultiByteToWideChar(CP_UTF8,
+                      0,
+                      strView.data(),
+                      static_cast<int>(strView.size()),
+                      &wstrTo[0],
+                      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<int>(wstr.size()),
+                                        nullptr,
+                                        0,
+                                        nullptr,
+                                        nullptr);
+  std::string strTo(size_needed, 0);
+  WideCharToMultiByte(CP_UTF8,
+                      0,
+                      &wstr[0],
+                      static_cast<int>(wstr.size()),
+                      &strTo[0],
+                      size_needed,
+                      nullptr,
+                      nullptr);
+  return strTo;
+}
+#endif  // _WIN32
+
 }  // namespace node
diff --git a/src/util.h b/src/util.h
@@ -49,6 +49,11 @@
 #include <unordered_map>
 #include <utility>
 #include <vector>
+#ifdef _WIN32
+#include <windows.h>
+#else
+#include <unistd.h>
+#endif
 
 #ifdef __GNUC__
 #define MUST_USE_RESULT __attribute__((warn_unused_result))
@@ -1013,6 +1018,30 @@ v8::Maybe<int> GetValidFileMode(Environment* env,
 // case insensitive.
 inline bool IsWindowsBatchFile(const char* filename);
 
+#ifdef _WIN32
+std::wstring ConvertToWideString(const std::string_view str);
+
+inline std::filesystem::path StringViewToPath(std::string_view str) {
+  return std::filesystem::path(ConvertToWideString(str));
+}
+
+std::string ConvertWideToUTF8(const std::wstring& wstr);
+
+inline std::string PathToString(std::filesystem::path path) {
+  return ConvertWideToUTF8(path.wstring());
+}
+
+#else  // _WIN32
+
+inline std::filesystem::path StringViewToPath(std::string_view str) {
+  return std::filesystem::path(str);
+}
+inline std::string PathToString(std::filesystem::path path) {
+  return path.native();
+}
+
+#endif  // _WIN32
+
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

diff --git a/test/fixtures/全角文字/index.js b/test/fixtures/全角文字/index.js
@@ -0,0 +1 @@
+console.log('check non-ascii');
diff --git a/test/parallel/test-non-ascii.js b/test/parallel/test-non-ascii.js
@@ -0,0 +1,12 @@
+'use strict';
+
+const common = require('../common');
+const path = require('path');
+const { test } = require('node:test');
+const assert = require('node:assert');
+
+test('Running from a directory with non-ASCII characters', async () => {
+  const nonAsciiPath = path.resolve(__dirname, '../fixtures/全角文字/index.js');
+  const { stdout } = await common.spawnPromisified(process.execPath, [nonAsciiPath]);
+  assert.strictEqual(stdout, 'check non-ascii\n');
+});
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		console.log('check non-ascii');
Copy link Contributor Author islandryu Nov 18, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. During testing, it became evident that the occurrence of the error described in issue #55773 is influenced by the character encoding used by the user. For instance, users working with CP1252 are unlikely to encounter any errors, regardless of the characters they use. In contrast, users with encodings like CP932 are more prone to experiencing errors. Take the character "月" as an example, which is represented in UTF-8 as: E6 9C 88 In CP932, the byte 88 is interpreted as the leading byte of a 2-byte character, potentially causing issues. The key takeaway is that to ensure accurate regression testing, it might be necessary to test the runtime environment with character encodings other than CP1252. I also looked for other tests that might include regression testing against Windows-specific character encodings but couldn’t find any. If you have any good ideas, I’d appreciate your input. Copy link Contributor huseyinacacak-janea Nov 19, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. From what I understand, you’re looking for something along these lines: const stdoutExec = execSync('@chcp 932 >nul & \"' + process.execPath + '\" \"' + nonAsciiPath + '\"', { encoding: 'utf8'}); assert.strictEqual(stdoutExec, 'check non-ascii\n'); However, I wanted to point out that this code does not fail when run on the main branch on my local machine. Because of this, it might not be ideal to use this approach directly without further investigation. Copy link Contributor Author islandryu Nov 20, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Thank you for your response. I tried it as well, and it seems that the behavior was not affected. It appears that we need to change the system's character encoding rather than just the console's encoding. For instance, the following minimal C++ code reproduces the same error: int main() { std::string path = "\x88"; std::filesystem::path file(path); } This code alone triggers the same issue. Copy link Contributor Author islandryu Nov 20, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. I am currently investigating whether it is possible to change the system's character encoding within a cctest.