Skip to content

[stable-3.16] fix(filesystem): ensure we set a file read-only or read-write on Windows #8215

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.

Sign up for GitHub

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

Merged
mgallien merged 5 commits into from
Apr 28, 2025
Merged

[stable-3.16] fix(filesystem): ensure we set a file read-only or read-write on Windows #8215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b5a67fe
fix(filesystem): log more permissions when deleting a file fails
mgallien Apr 25, 2025
3f57fa1
fix(filesystem): use platform specific API to make a file read-only
mgallien Apr 25, 2025
6f97ee8
fix(propagation): ensure we delete pending folders before terminating
mgallien Apr 25, 2025
30db63b
fix(propagation): more logs on folder deletions propagation step
mgallien Apr 25, 2025
b6d7450
fix(propagation): ensure we run file removal propagation steps
mgallien Apr 25, 2025
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
87 changes: 62 additions & 25 deletions src/common/filesystembase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,36 +111,43 @@ static QFile::Permissions getDefaultWritePermissions()
void FileSystem::setFileReadOnly(const QString &filename, bool readonly)
{
#ifdef Q_OS_WIN
if (isLnkFile(filename)) {
if (!fileExists(filename)) {
return;
}
try {
const auto permissions = filePermissionsWin(filename);
if (!fileExists(filename)) {
Q_ASSERT(false);
return;
}

std::filesystem::perms allWritePermissions = std::filesystem::perms::_All_write;
static std::filesystem::perms defaultWritePermissions = std::filesystem::perms::others_write;
const auto fileAttributes = GetFileAttributesW(filename.toStdWString().c_str());
if (fileAttributes == INVALID_FILE_ATTRIBUTES) {
const auto lastError = GetLastError();
auto errorMessage = static_cast<char*>(nullptr);
if (FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
nullptr, lastError, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), errorMessage, 0, nullptr) == 0) {
qCWarning(lcFileSystem()) << "GetFileAttributesW" << filename << (readonly ? "readonly" : "read write") << errorMessage;
} else {
qCWarning(lcFileSystem()) << "GetFileAttributesW" << filename << (readonly ? "readonly" : "read write") << "unknown error" << lastError;
}
return;
}

std::filesystem::permissions(filename.toStdWString(), allWritePermissions, std::filesystem::perm_options::remove);
auto newFileAttributes = fileAttributes;
if (readonly) {
newFileAttributes = newFileAttributes | FILE_ATTRIBUTE_READONLY;
} else {
newFileAttributes = newFileAttributes & (~FILE_ATTRIBUTE_READONLY);
}

if (!readonly) {
std::filesystem::permissions(filename.toStdWString(), defaultWritePermissions, std::filesystem::perm_options::add);
}
}
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcFileSystem()) << filename << (readonly ? "readonly" : "read write") << e.what();
}
catch (const std::system_error &e)
{
qCWarning(lcFileSystem()) << filename << e.what();
}
catch (...)
{
qCWarning(lcFileSystem()) << filename;
if (SetFileAttributesW(filename.toStdWString().c_str(), newFileAttributes) == 0) {
const auto lastError = GetLastError();
auto errorMessage = static_cast<char*>(nullptr);
if (FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
nullptr, lastError, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), errorMessage, 0, nullptr) == 0) {
qCWarning(lcFileSystem()) << "SetFileAttributesW" << filename << (readonly ? "readonly" : "read write") << errorMessage;
} else {
qCWarning(lcFileSystem()) << "SetFileAttributesW" << filename << (readonly ? "readonly" : "read write") << "unknown error" << lastError;
}
return;
}

return;
#endif
QFile file(filename);
QFile::Permissions permissions = file.permissions();
Expand Down Expand Up @@ -320,6 +327,11 @@ bool FileSystem::openAndSeekFileSharedRead(QFile *file, QString *errorOrNull, qi
}

#ifdef Q_OS_WIN
std::filesystem::perms FileSystem::filePermissionsWinSymlinkSafe(const QString &filename)
{
return std::filesystem::symlink_status(filename.toStdWString()).permissions();
}

std::filesystem::perms FileSystem::filePermissionsWin(const QString &filename)
{
return std::filesystem::status(filename.toStdWString()).permissions();
Expand Down Expand Up @@ -551,6 +563,31 @@ bool FileSystem::remove(const QString &fileName, QString *errorString)
*errorString = f.errorString();
}
qCWarning(lcFileSystem()) << f.errorString() << fileName;

#if defined Q_OS_WIN
const auto permissionsDisplayHelper = [] (std::filesystem::perms currentPermissions) {
const auto unitaryHelper = [currentPermissions] (std::filesystem::perms testedPermission, char permissionChar) {
return (static_cast<bool>(currentPermissions & testedPermission) ? permissionChar : '-');
};

qCInfo(lcFileSystem()) << unitaryHelper(std::filesystem::perms::owner_read, 'r')
<< unitaryHelper(std::filesystem::perms::owner_write, 'w')
<< unitaryHelper(std::filesystem::perms::owner_exec, 'x')
<< unitaryHelper(std::filesystem::perms::group_read, 'r')
<< unitaryHelper(std::filesystem::perms::group_write, 'w')
<< unitaryHelper(std::filesystem::perms::group_exec, 'x')
<< unitaryHelper(std::filesystem::perms::others_read, 'r')
<< unitaryHelper(std::filesystem::perms::others_write, 'w')
<< unitaryHelper(std::filesystem::perms::others_exec, 'x');
};

const auto unsafeFilePermissions = filePermissionsWin(fileName);
permissionsDisplayHelper(unsafeFilePermissions);

const auto safeFilePermissions = filePermissionsWinSymlinkSafe(fileName);
permissionsDisplayHelper(safeFilePermissions);
#endif

return false;
}
return true;
Expand Down
1 change: 1 addition & 0 deletions src/common/filesystembase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ namespace FileSystem {
*/
QString OCSYNC_EXPORT pathtoUNC(const QString &str);

std::filesystem::perms OCSYNC_EXPORT filePermissionsWinSymlinkSafe(const QString &filename);
std::filesystem::perms OCSYNC_EXPORT filePermissionsWin(const QString &filename);
void OCSYNC_EXPORT setFilePermissionsWin(const QString &filename, const std::filesystem::perms &perms);
#endif
Expand Down
54 changes: 39 additions & 15 deletions src/libsync/owncloudpropagator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,7 @@ PropagatorJob::JobParallelism PropagateDirectory::parallelism() const
bool PropagateDirectory::scheduleSelfOrChild()
{
if (_state == Finished) {
qCDebug(lcDirectory) << "folder job finished";
return false;
}

Expand All @@ -1374,15 +1375,32 @@ bool PropagateDirectory::scheduleSelfOrChild()
}

if (_firstJob && _firstJob->_state == NotYetStarted) {
return _firstJob->scheduleSelfOrChild();
const auto result = _firstJob->scheduleSelfOrChild();

if (result) {
qCDebug(lcDirectory) << "folder first job has more work to do";
} else {
qCDebug(lcDirectory) << "folder first job is done";
}

return result;
}

if (_firstJob && _firstJob->_state == Running) {
// Don't schedule any more job until this is done.
qCDebug(lcDirectory) << "first job is running";
return false;
}

return _subJobs.scheduleSelfOrChild();
const auto result = _subJobs.scheduleSelfOrChild();

if (result) {
qCDebug(lcDirectory) << "folder child jobs have more work to do";
} else {
qCDebug(lcDirectory) << "folder child jobs are done";
}

return result;
}

void PropagateDirectory::slotFirstJobFinished(SyncFileItem::Status status)
Expand Down Expand Up @@ -1527,6 +1545,7 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
}
}
_state = Finished;
qCDebug(lcDirectory()) << "PropagateDirectory::slotSubJobsFinished" << "emit finished" << status;
emit finished(status);
}

Expand Down Expand Up @@ -1579,28 +1598,36 @@ qint64 PropagateRootDirectory::committedDiskSpace() const

void PropagateRootDirectory::appendDirDeletionJob(PropagatorJob *job)
{
if (auto directoryJob = qobject_cast<PropagateDirectory*>(job)) {
qCDebug(lcRootDirectory) << "new folder deletion job" << directoryJob->_item->_file;
}
_dirDeletionJobs.appendJob(job);
}

bool PropagateRootDirectory::scheduleSelfOrChild()
{
if (_state == Finished) {
qCDebug(lcRootDirectory) << "root folder fully propagated";
return false;
}

if (PropagateDirectory::scheduleSelfOrChild() && propagator()->delayedTasks().empty()) {
qCDebug(lcRootDirectory) << "root folder has more jobs to do";
return true;
}

// Important: Finish _subJobs before scheduling any deletes.
if (_subJobs._state != Finished) {
qCDebug(lcRootDirectory) << "root folder has running jobs to do";
return false;
}

if (!propagator()->delayedTasks().empty()) {
qCDebug(lcRootDirectory) << "root folder has more delayed jobs to do";
return scheduleDelayedJobs();
}

qCDebug(lcRootDirectory) << "schedule folder deletions step";
return _dirDeletionJobs.scheduleSelfOrChild();
}

Expand All @@ -1611,15 +1638,12 @@ void PropagateRootDirectory::slotSubJobsFinished(SyncFileItem::Status status)
return;
}

if (status != SyncFileItem::Success
&& status != SyncFileItem::Restoration
&& status != SyncFileItem::BlacklistedError
&& status != SyncFileItem::FileNameClash
&& status != SyncFileItem::Conflict) {
if (status == SyncFileItem::FatalError) {
if (_state != Finished) {
// Synchronously abort
abort(AbortType::Synchronous);
_state = Finished;
qCInfo(lcRootDirectory()) << "PropagateRootDirectory::slotSubJobsFinished" << "emit finished" << status;
emit finished(status);
}
return;
Expand All @@ -1628,18 +1652,18 @@ void PropagateRootDirectory::slotSubJobsFinished(SyncFileItem::Status status)
if (_errorStatus == SyncFileItem::NoStatus) {
switch (status) {
case SyncFileItem::NoStatus:
case SyncFileItem::FatalError:
case SyncFileItem::NormalError:
case SyncFileItem::SoftError:
case SyncFileItem::Conflict:
case SyncFileItem::FileIgnored:
case SyncFileItem::FileLocked:
case SyncFileItem::Restoration:
case SyncFileItem::FileNameInvalid:
case SyncFileItem::FileNameInvalidOnServer:
case SyncFileItem::DetailError:
case SyncFileItem::Success:
break;
case SyncFileItem::FileLocked:
case SyncFileItem::DetailError:
case SyncFileItem::SoftError:
case SyncFileItem::Conflict:
case SyncFileItem::FatalError:
case SyncFileItem::FileNameInvalid:
case SyncFileItem::FileNameInvalidOnServer:
case SyncFileItem::NormalError:
case SyncFileItem::FileNameClash:
case SyncFileItem::BlacklistedError:
_errorStatus = status;
Expand Down