fs: fix rmSync to handle non-ASCII characters

src/api/exceptions.cc

@@ -20,6 +20,21 @@ using v8::Object;
 using v8::String;
 using v8::Value;
 
+static Local<String> StringFromPath(Isolate* isolate, const char* path) {
+#ifdef _WIN32
+  if (strncmp(path, "\\\\?\\UNC\\", 8) == 0) {
+    return String::Concat(
+        isolate,
+        FIXED_ONE_BYTE_STRING(isolate, "\\\\"),
+        String::NewFromUtf8(isolate, path + 8).ToLocalChecked());
+  } else if (strncmp(path, "\\\\?\\", 4) == 0) {
+    return String::NewFromUtf8(isolate, path + 4).ToLocalChecked();
+  }
+#endif
+
+  return String::NewFromUtf8(isolate, path).ToLocalChecked();
+}
+
 Local<Value> ErrnoException(Isolate* isolate,
                             int errorno,
                             const char* syscall,
@@ -41,8 +56,7 @@ Local<Value> ErrnoException(Isolate* isolate,
 
   Local<String> path_string;
   if (path != nullptr) {
-    // FIXME(bnoordhuis) It's questionable to interpret the file path as UTF-8.
-    path_string = String::NewFromUtf8(isolate, path).ToLocalChecked();
+    path_string = StringFromPath(isolate, path);
   }
 
   if (path_string.IsEmpty() == false) {
@@ -72,22 +86,6 @@ Local<Value> ErrnoException(Isolate* isolate,
   return e;
 }
 
-static Local<String> StringFromPath(Isolate* isolate, const char* path) {
-#ifdef _WIN32
-  if (strncmp(path, "\\\\?\\UNC\\", 8) == 0) {
-    return String::Concat(
-        isolate,
-        FIXED_ONE_BYTE_STRING(isolate, "\\\\"),
-        String::NewFromUtf8(isolate, path + 8).ToLocalChecked());
-  } else if (strncmp(path, "\\\\?\\", 4) == 0) {
-    return String::NewFromUtf8(isolate, path + 4).ToLocalChecked();
-  }
-#endif
-
-  return String::NewFromUtf8(isolate, path).ToLocalChecked();
-}
-
-
 Local<Value> UVException(Isolate* isolate,
                          int errorno,
                          const char* syscall,

src/node_file.cc

@@ -1614,6 +1614,55 @@ static void RMDir(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
+#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 RmSync(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();
@@ -1625,7 +1674,7 @@ static void RmSync(const FunctionCallbackInfo<Value>& args) {
   ToNamespacedPath(env, &path);
   THROW_IF_INSUFFICIENT_PERMISSIONS(
       env, permission::PermissionScope::kFileSystemWrite, path.ToStringView());
-  auto file_path = std::filesystem::path(path.ToStringView());
+  auto file_path = BufferValueToPath(path);
   std::error_code error;
   auto file_status = std::filesystem::status(file_path, error);
 
@@ -3181,6 +3230,106 @@ std::string ConvertWideToUTF8(const std::wstring& wstr) {
 
 #endif  // _WIN32
 
+static void RmSync(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  Isolate* isolate = env->isolate();
+
+  CHECK_EQ(args.Length(), 4);  // path, maxRetries, recursive, retryDelay
+
+  BufferValue path(isolate, args[0]);
+  CHECK_NOT_NULL(*path);
+  ToNamespacedPath(env, &path);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFileSystemWrite, path.ToStringView());
+  auto file_path = BufferValueToPath(path);
+  std::error_code error;
+  auto file_status = std::filesystem::status(file_path, error);
+
+  if (file_status.type() == std::filesystem::file_type::not_found) {
+    return;
+  }
+
+  int maxRetries = args[1].As<Int32>()->Value();
+  int recursive = args[2]->IsTrue();
+  int retryDelay = args[3].As<Int32>()->Value();
+
+  // File is a directory and recursive is false
+  if (file_status.type() == std::filesystem::file_type::directory &&
+      !recursive) {
+    auto file_path_as_str = PathToString(file_path);
+    return THROW_ERR_FS_EISDIR(
+        isolate, "Path is a directory: %s", file_path_as_str);
+  }
+
+  // Allowed errors are:
+  // - EBUSY: std::errc::device_or_resource_busy
+  // - EMFILE: std::errc::too_many_files_open
+  // - ENFILE: std::errc::too_many_files_open_in_system
+  // - ENOTEMPTY: std::errc::directory_not_empty
+  // - EPERM: std::errc::operation_not_permitted
+  auto can_omit_error = [](std::error_code error) -> bool {
+    return (error == std::errc::device_or_resource_busy ||
+            error == std::errc::too_many_files_open ||
+            error == std::errc::too_many_files_open_in_system ||
+            error == std::errc::directory_not_empty ||
+            error == std::errc::operation_not_permitted);
+  };
+
+  int i = 1;
+
+  while (maxRetries >= 0) {
+    if (recursive) {
+      std::filesystem::remove_all(file_path, error);
+    } else {
+      std::filesystem::remove(file_path, error);
+    }
+
+    if (!error || error == std::errc::no_such_file_or_directory) {
+      return;
+    } else if (!can_omit_error(error)) {
+      break;
+    }
+
+    if (retryDelay > 0) {
+#ifdef _WIN32
+      Sleep(i * retryDelay / 1000);
+#else
+      sleep(i * retryDelay / 1000);
+#endif
+    }
+    maxRetries--;
+    i++;
+  }
+
+  // On Windows path::c_str() returns wide char, convert to std::string first.
+  std::string file_path_str = PathToString(file_path);
+  const char* path_c_str = file_path_str.c_str();
+#ifdef _WIN32
+  int permission_denied_error = EPERM;
+#else
+  int permission_denied_error = EACCES;
+#endif  // !_WIN32
+
+  if (error == std::errc::operation_not_permitted) {
+    std::string message = "Operation not permitted: ";
+    return env->ThrowErrnoException(EPERM, "rm", message.c_str(), path_c_str);
+  } else if (error == std::errc::directory_not_empty) {
+    std::string message = "Directory not empty: ";
+    return env->ThrowErrnoException(EACCES, "rm", message.c_str(), path_c_str);
+  } else if (error == std::errc::not_a_directory) {
+    std::string message = "Not a directory: ";
+    return env->ThrowErrnoException(ENOTDIR, "rm", message.c_str(), path_c_str);
+  } else if (error == std::errc::permission_denied) {
+    std::string message = "Permission denied: ";
+    return env->ThrowErrnoException(
+        permission_denied_error, "rm", message.c_str(), path_c_str);
+  }
+
+  std::string message = "Unknown error: " + error.message();
+  return env->ThrowErrnoException(
+      UV_UNKNOWN, "rm", message.c_str(), path_c_str);
+}
+
 static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();

test/parallel/test-fs-rmSync-special-char-additional-error.js

@@ -0,0 +1,48 @@
+'use strict';
+require('../common');
+const tmpdir = require('../common/tmpdir');
+const assert = require('node:assert');
+const fs = require('node:fs');
+const path = require('node:path');
+const { execSync } = require('child_process');
+
+tmpdir.refresh(); // Prepare a clean temporary directory
+
+const dirPath = path.join(tmpdir.path, '速_dir');
+const filePath = path.join(dirPath, 'test_file.txt');
+
+// Create a directory and a file within it
+fs.mkdirSync(dirPath, { recursive: true });
+fs.writeFileSync(filePath, 'This is a test file.');
+
+// Set permissions to simulate a permission denied scenario
+if (process.platform === 'win32') {
+  // Windows: Deny delete permissions
+  execSync(`icacls "${filePath}" /deny Everyone:(D)`);
+} else {
+  // Unix/Linux: Remove write permissions from the directory
+  fs.chmodSync(dirPath, 0o555); // Read and execute permissions only
+}
+
+// Attempt to delete the directory which should now fail
+try {
+  fs.rmSync(dirPath, { recursive: true });
+} catch (err) {
+  // Verify that the error is due to permission restrictions
+  const expectedCode = process.platform === 'win32' ? 'EPERM' : 'EACCES';
+  assert.strictEqual(err.code, expectedCode);
+  assert.strictEqual(err.path, dirPath);
+  assert(err.message.includes(dirPath), 'Error message should include the path treated as a directory');
+}
+
+// Cleanup - resetting permissions and removing the directory safely
+if (process.platform === 'win32') {
+  // Remove the explicit permissions before attempting to delete
+  execSync(`icacls "${filePath}" /remove:d Everyone`);
+} else {
+  // Reset permissions to allow deletion
+  fs.chmodSync(dirPath, 0o755); // Restore full permissions to the directory
+}
+
+// Attempt to clean up
+fs.rmSync(dirPath, { recursive: true }); // This should now succeed

test/parallel/test-fs-rmSync-special-char.js

@@ -0,0 +1,32 @@
+'use strict';
+require('../common');
+const tmpdir = require('../common/tmpdir');
+const assert = require('node:assert');
+const fs = require('node:fs');
+const path = require('node:path');
+
+// This test ensures that fs.rmSync handles non-ASCII characters in file paths,
+// and that errors contain correctly encoded paths and err.path values.
+
+tmpdir.refresh(); // Prepare a clean temporary directory
+
+// Define paths with non-ASCII characters
+const dirPath = path.join(tmpdir.path, '速_dir');
+const filePath = path.join(tmpdir.path, '速.txt');
+
+// Create a directory and a file with non-ASCII characters
+fs.mkdirSync(dirPath);
+fs.writeFileSync(filePath, 'This is a test file with special characters.');
+fs.rmSync(filePath);
+assert.strictEqual(fs.existsSync(filePath), false);
+
+// Ensure rmSync throws an error when trying to remove a directory without recursive
+assert.throws(() => {
+  fs.rmSync(dirPath, { recursive: false });
+}, (err) => {
+  // Assert the error code and check that the error message includes the correct non-ASCII path
+  assert.strictEqual(err.code, 'ERR_FS_EISDIR');
+  assert(err.message.includes(dirPath), 'Error message should include the directory path');
+  assert.strictEqual(err.path, dirPath);
+  return true;
+});