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 6 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
104 changes: 53 additions & 51 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 @@ -1637,11 +1686,13 @@ static void RmSync(const FunctionCallbackInfo<Value>& args) {
int recursive = args[2]->IsTrue();
int retryDelay = args[3].As<Int32>()->Value();

auto file_path_as_str = PathToString(file_path);
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved

// File is a directory and recursive is false
if (file_status.type() == std::filesystem::file_type::directory &&
!recursive) {
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 @@ -3125,55 +3176,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
36 changes: 36 additions & 0 deletions test/parallel/test-fs-rmSync-special-char.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'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 should not throw an error for non-ASCII file names
fs.rmSync(filePath);

// Ensure the file has been removed
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;
});
Loading