Skip to content

Commit

Permalink
addressred reviewrs suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
Yeaseen committed Jan 19, 2025
1 parent fa8ef5a commit dc5fd32
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 5 deletions.
4 changes: 2 additions & 2 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1686,11 +1686,11 @@ 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);

// 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);
}
Expand Down Expand Up @@ -1736,7 +1736,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
13 changes: 10 additions & 3 deletions test/parallel/test-fs-rmSync-special-char.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,13 @@ 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 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
Expand All @@ -34,3 +32,12 @@ assert.throws(() => {
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;
});

0 comments on commit dc5fd32

Please sign in to comment.