fs: fix rmSync error messages for non-ASCII paths

src/api/exceptions.cc

@@ -8,6 +8,8 @@
 #include "v8.h"
 
 #include <cstring>
+#include <string>
+#include <string_view>
 
 namespace node {
 
@@ -20,6 +22,40 @@ using v8::Object;
 using v8::String;
 using v8::Value;
 
+static Local<String> StringFromPath(Isolate* isolate, std::string_view path) {
+#ifdef _WIN32
+  constexpr std::string_view kUncPrefix = "\\\\?\\UNC\\";
+  constexpr std::string_view kLongPrefix = "\\\\?\\";
+
+  if (path.starts_with(kUncPrefix)) {
+    std::string s;
+    s.reserve(2 + (path.size() - kUncPrefix.size()));
+    s.append("\\\\");
+    s.append(path.substr(kUncPrefix.size()));
+    return String::NewFromUtf8(isolate,
+                               s.data(),
+                               v8::NewStringType::kNormal,
+                               static_cast<int>(s.size()))
+        .ToLocalChecked();
+  }
+
+  if (path.starts_with(kLongPrefix)) {
+    auto rest = path.substr(kLongPrefix.size());
+    return String::NewFromUtf8(isolate,
+                               rest.data(),
+                               v8::NewStringType::kNormal,
+                               static_cast<int>(rest.size()))
+        .ToLocalChecked();
+  }
+#endif
+
+  return String::NewFromUtf8(isolate,
+                             path.data(),
+                             v8::NewStringType::kNormal,
+                             static_cast<int>(path.size()))
+      .ToLocalChecked();
+}
+
 Local<Value> ErrnoException(Isolate* isolate,
                             int errorno,
                             const char* syscall,
@@ -42,7 +78,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, std::string_view(path));
   }
 
   if (path_string.IsEmpty() == false) {
@@ -72,22 +108,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,
@@ -114,7 +134,7 @@ Local<Value> UVException(Isolate* isolate,
   js_msg = String::Concat(isolate, js_msg, js_syscall);
 
   if (path != nullptr) {
-    js_path = StringFromPath(isolate, path);
+    js_path = StringFromPath(isolate, std::string_view(path));
 
     js_msg =
         String::Concat(isolate, js_msg, FIXED_ONE_BYTE_STRING(isolate, " '"));
@@ -124,7 +144,7 @@ Local<Value> UVException(Isolate* isolate,
   }
 
   if (dest != nullptr) {
-    js_dest = StringFromPath(isolate, dest);
+    js_dest = StringFromPath(isolate, std::string_view(dest));
 
     js_msg = String::Concat(
         isolate, js_msg, FIXED_ONE_BYTE_STRING(isolate, " -> '"));

src/node_file.cc

@@ -1828,7 +1828,7 @@ static void RmSync(const FunctionCallbackInfo<Value>& args) {
   }
 
   // On Windows path::c_str() returns wide char, convert to std::string first.
-  std::string file_path_str = file_path.string();
+  std::string file_path_str = ConvertPathToUTF8(file_path);
   const char* path_c_str = file_path_str.c_str();
 #ifdef _WIN32
   int permission_denied_error = EPERM;
@@ -1837,14 +1837,14 @@ static void RmSync(const FunctionCallbackInfo<Value>& args) {
 #endif  // !_WIN32
 
   if (error == std::errc::operation_not_permitted) {
-    std::string message = "Operation not permitted: " + file_path_str;
+    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: " + file_path_str;
+    std::string message = "Directory not empty:";
     return env->ThrowErrnoException(
         ENOTEMPTY, "rm", message.c_str(), path_c_str);
   } else if (error == std::errc::not_a_directory) {
-    std::string message = "Not a directory: " + file_path_str;
+    std::string message = "Not a directory:";
     return env->ThrowErrnoException(ENOTDIR, "rm", message.c_str(), path_c_str);
 #ifdef _AIX
   } else if (error == std::errc::permission_denied ||
@@ -1855,7 +1855,7 @@ static void RmSync(const FunctionCallbackInfo<Value>& args) {
 #else
   } else if (error == std::errc::permission_denied) {
 #endif
-    std::string message = "Permission denied: " + file_path_str;
+    std::string message = "Permission denied:";
     return env->ThrowErrnoException(
         permission_denied_error, "rm", message.c_str(), path_c_str);
   }

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

@@ -0,0 +1,69 @@
+'use strict';
+const common = 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
+
+function countOccurrences(haystack, needle) {
+  let count = 0;
+  let offset = 0;
+
+  while ((offset = haystack.indexOf(needle, offset)) !== -1) {
+    count++;
+    offset += needle.length;
+  }
+
+  return count;
+}
+
+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
+}
+
+try {
+  // Attempt to delete the directory which should now fail
+  assert.throws(() => {
+    fs.rmSync(dirPath, { recursive: true });
+  }, (err) => {
+    // Verify that the error is due to permission restrictions
+    let expectedCode = 'EACCES';
+    if (common.isMacOS) {
+      expectedCode = 'ENOTEMPTY';
+    } else if (common.isWindows) {
+      expectedCode = 'EPERM';
+    }
+    assert.strictEqual(err.code, expectedCode);
+    assert.strictEqual(err.syscall, 'rm');
+    assert.strictEqual(err.path, dirPath);
+    assert.strictEqual(countOccurrences(err.message, dirPath), 1);
+    return true;
+  });
+} finally {
+  // 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
+}