Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: fix rmSync to handle non-ASCII characters #56117

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 53 additions & 52 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Yeaseen marked this conversation as resolved.
Show resolved Hide resolved
std::error_code error;
auto file_status = std::filesystem::status(file_path, error);

Expand All @@ -1640,8 +1689,9 @@ static void RmSync(const FunctionCallbackInfo<Value>& args) {
// 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.c_str());
isolate, "Path is a directory: %s", file_path_as_str);
}

// Allowed errors are:
Expand Down Expand Up @@ -1685,7 +1735,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 = PathToString(file_path);
const char* path_c_str = file_path_str.c_str();
#ifdef _WIN32
int permission_denied_error = EPERM;
Expand Down Expand Up @@ -3145,55 +3195,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();
Expand Down
43 changes: 43 additions & 0 deletions test/parallel/test-fs-rmSync-special-char.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
'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');
const incorrectDirPath = filePath + path.sep; // Treat file as if it were a directory

// 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 });
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
}, (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;
});

// Test for not_a_directory error when a file path is incorrectly treated as a directory path
assert.throws(() => {
fs.rmSync(incorrectDirPath, { recursive: true });
}, (err) => {
assert.strictEqual(err.code, 'ENOENT');
assert(err.message.includes(incorrectDirPath), 'Error message should include the path treated as a directory');
return true;
});
Loading