Skip to content

Commit cd10114

Browse files
SparshGarg999aduh95
authored andcommitted
fs: support removing read-only files in rmSync on Windows
On Windows, libc++ std::filesystem::remove and remove_all do not automatically clear the read-only attribute before deleting a file (unlike MSVC STL). This causes fs.rmSync to fail with EPERM when trying to remove read-only files in environments where Node.js is built using clang libc++ (such as Electron). This commit introduces a Windows-specific helper ClearReadOnlyAttributeW which clears the FILE_ATTRIBUTE_READONLY attribute recursively (or for a single file) when operation_not_permitted is returned, allowing rmSync to successfully delete read-only files/folders. Fixes: #64374 Signed-off-by: SparshGarg999 <[email protected]> PR-URL: #64453 Fixes: #64374 Reviewed-By: Stefan Stojanovic <[email protected]>
1 parent ac37bbe commit cd10114

2 files changed

Lines changed: 81 additions & 0 deletions

File tree

‎src/node_file.cc‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1745,6 +1745,41 @@ static void RMDir(const FunctionCallbackInfo<Value>& args) {
17451745
}
17461746
}
17471747

1748+
#ifdef _WIN32
1749+
static void ClearReadOnlyAttributeWHelper(const wchar_t* path) {
1750+
DWORD attrs = GetFileAttributesW(path);
1751+
if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_READONLY)) {
1752+
SetFileAttributesW(path, attrs & ~FILE_ATTRIBUTE_READONLY);
1753+
}
1754+
}
1755+
1756+
static void ClearReadOnlyAttributeW(const std::filesystem::path& path,
1757+
bool recursive) {
1758+
std::error_code ec;
1759+
auto file_status = std::filesystem::symlink_status(path, ec);
1760+
if (ec) return;
1761+
1762+
if (recursive &&
1763+
file_status.type() == std::filesystem::file_type::directory) {
1764+
for (const auto& entry : std::filesystem::recursive_directory_iterator(
1765+
path,
1766+
std::filesystem::directory_options::skip_permission_denied,
1767+
ec)) {
1768+
std::error_code entry_ec;
1769+
auto entry_status = entry.symlink_status(entry_ec);
1770+
if (entry_ec) continue;
1771+
if (entry_status.type() != std::filesystem::file_type::symlink) {
1772+
ClearReadOnlyAttributeWHelper(entry.path().c_str());
1773+
}
1774+
}
1775+
}
1776+
1777+
if (file_status.type() != std::filesystem::file_type::symlink) {
1778+
ClearReadOnlyAttributeWHelper(path.c_str());
1779+
}
1780+
}
1781+
#endif
1782+
17481783
static void RmSync(const FunctionCallbackInfo<Value>& args) {
17491784
Environment* env = Environment::GetCurrent(args);
17501785
Isolate* isolate = env->isolate();
@@ -1792,6 +1827,9 @@ static void RmSync(const FunctionCallbackInfo<Value>& args) {
17921827
};
17931828

17941829
int i = 1;
1830+
#ifdef _WIN32
1831+
bool cleared_readonly = false;
1832+
#endif
17951833

17961834
while (maxRetries >= 0) {
17971835
if (recursive) {
@@ -1800,6 +1838,22 @@ static void RmSync(const FunctionCallbackInfo<Value>& args) {
18001838
std::filesystem::remove(file_path, error);
18011839
}
18021840

1841+
#ifdef _WIN32
1842+
// On Windows, libc++ does not clear the read-only attribute before
1843+
// removing a file (unlike MSVC STL which does). Attempt to clear it
1844+
// manually when we get EPERM (operation_not_permitted) so that read-only
1845+
// files can be deleted, matching the behavior of official Node.js builds.
1846+
if (error == std::errc::operation_not_permitted && !cleared_readonly) {
1847+
cleared_readonly = true;
1848+
ClearReadOnlyAttributeW(file_path, recursive);
1849+
if (recursive) {
1850+
std::filesystem::remove_all(file_path, error);
1851+
} else {
1852+
std::filesystem::remove(file_path, error);
1853+
}
1854+
}
1855+
#endif // _WIN32
1856+
18031857
if (!error || error == std::errc::no_such_file_or_directory) {
18041858
return;
18051859
} else if (!can_omit_error(error)) {

‎test/parallel/test-fs-rm.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,3 +631,30 @@ if (isGitPresent) {
631631
}
632632
}
633633
}
634+
635+
{
636+
// Test that rmSync can delete read-only files (and directories containing read-only files recursively)
637+
const dirname = nextDirPath();
638+
const filePath = path.join(dirname, 'readonly-file.txt');
639+
const recursiveDir = path.join(dirname, 'subdir');
640+
const recursiveFilePath = path.join(recursiveDir, 'readonly-nested.txt');
641+
642+
fs.mkdirSync(recursiveDir, { recursive: true });
643+
fs.writeFileSync(filePath, 'hello');
644+
fs.writeFileSync(recursiveFilePath, 'world');
645+
646+
// Make files read-only
647+
fs.chmodSync(filePath, 0o444);
648+
fs.chmodSync(recursiveFilePath, 0o444);
649+
650+
// rmSync without recursive option on a file
651+
fs.rmSync(filePath);
652+
assert.strictEqual(fs.existsSync(filePath), false);
653+
654+
// rmSync with recursive option on a directory containing a read-only file
655+
fs.rmSync(recursiveDir, { recursive: true });
656+
assert.strictEqual(fs.existsSync(recursiveDir), false);
657+
658+
// Clean up parent directory
659+
fs.rmSync(dirname, { recursive: true, force: true });
660+
}

0 commit comments

Comments
 (0)