Skip to content

Commit 2987a59

Browse files
SparshGarg999SparshGarg999
authored
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 <sparshgarg999@gmail.com> PR-URL: #64453 Fixes: #64374 Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
1 parent be41ec7 commit 2987a59

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
@@ -1763,6 +1763,41 @@ static void RMDir(const FunctionCallbackInfo<Value>& args) {
17631763
}
17641764
}
17651765

1766+
#ifdef _WIN32
1767+
static void ClearReadOnlyAttributeWHelper(const wchar_t* path) {
1768+
DWORD attrs = GetFileAttributesW(path);
1769+
if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_READONLY)) {
1770+
SetFileAttributesW(path, attrs & ~FILE_ATTRIBUTE_READONLY);
1771+
}
1772+
}
1773+
1774+
static void ClearReadOnlyAttributeW(const std::filesystem::path& path,
1775+
bool recursive) {
1776+
std::error_code ec;
1777+
auto file_status = std::filesystem::symlink_status(path, ec);
1778+
if (ec) return;
1779+
1780+
if (recursive &&
1781+
file_status.type() == std::filesystem::file_type::directory) {
1782+
for (const auto& entry : std::filesystem::recursive_directory_iterator(
1783+
path,
1784+
std::filesystem::directory_options::skip_permission_denied,
1785+
ec)) {
1786+
std::error_code entry_ec;
1787+
auto entry_status = entry.symlink_status(entry_ec);
1788+
if (entry_ec) continue;
1789+
if (entry_status.type() != std::filesystem::file_type::symlink) {
1790+
ClearReadOnlyAttributeWHelper(entry.path().c_str());
1791+
}
1792+
}
1793+
}
1794+
1795+
if (file_status.type() != std::filesystem::file_type::symlink) {
1796+
ClearReadOnlyAttributeWHelper(path.c_str());
1797+
}
1798+
}
1799+
#endif
1800+
17661801
static void RmSync(const FunctionCallbackInfo<Value>& args) {
17671802
Environment* env = Environment::GetCurrent(args);
17681803
Isolate* isolate = env->isolate();
@@ -1810,6 +1845,9 @@ static void RmSync(const FunctionCallbackInfo<Value>& args) {
18101845
};
18111846

18121847
int i = 1;
1848+
#ifdef _WIN32
1849+
bool cleared_readonly = false;
1850+
#endif
18131851

18141852
while (maxRetries >= 0) {
18151853
if (recursive) {
@@ -1818,6 +1856,22 @@ static void RmSync(const FunctionCallbackInfo<Value>& args) {
18181856
std::filesystem::remove(file_path, error);
18191857
}
18201858

1859+
#ifdef _WIN32
1860+
// On Windows, libc++ does not clear the read-only attribute before
1861+
// removing a file (unlike MSVC STL which does). Attempt to clear it
1862+
// manually when we get EPERM (operation_not_permitted) so that read-only
1863+
// files can be deleted, matching the behavior of official Node.js builds.
1864+
if (error == std::errc::operation_not_permitted && !cleared_readonly) {
1865+
cleared_readonly = true;
1866+
ClearReadOnlyAttributeW(file_path, recursive);
1867+
if (recursive) {
1868+
std::filesystem::remove_all(file_path, error);
1869+
} else {
1870+
std::filesystem::remove(file_path, error);
1871+
}
1872+
}
1873+
#endif // _WIN32
1874+
18211875
if (!error || error == std::errc::no_such_file_or_directory) {
18221876
return;
18231877
} 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)