VladPolskiy/nextcloud-desktop
1
0
Fork 0
mirror of https://github.com/nextcloud/desktop.git synced 2025-07-21 17:32:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(filesystem): only change folder permissions if required

Browse Source 
On Linux changing the permissions causes inotify to create a IN_ATTRIB
event -- even if the permissions stays the same.

Such an event is passed to the filesystem watcher which lets the client
schedule a new sync run.  In certain conditions, this could happen
during every sync run...

Signed-off-by: Jyrki Gadinger <nilsding@nilsding.org>
This commit is contained in:
Jyrki Gadinger
2025-05-20 19:25:53 +02:00
committed by Matthieu Gallien
parent 65f4edb806
commit d9a0a6eab3
4 changed files with 184 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
src/libsync/filesystem.cpp
View File

@ -345,8 +345,15 @@ bool FileSystem::getInode(const QString &filename, quint64 *inode)
}
bool FileSystem::setFolderPermissions(const QString &path,
FileSystem::FolderPermissions permissions) noexcept
FileSystem::FolderPermissions permissions,
bool * const permissionsChanged) noexcept
{
bool permissionsDidChange = false;
if (permissionsChanged) {
*permissionsChanged = false;
}
#ifdef Q_OS_WIN
SECURITY_INFORMATION info = DACL_SECURITY_INFORMATION;
std::unique_ptr<char[]> securityDescriptor;
@ -476,19 +483,48 @@ bool FileSystem::setFolderPermissions(const QString &path,
qCWarning(lcFileSystem) << "error when calling SetFileSecurityW" << QDir::toNativeSeparators(path) << GetLastError();
return false;
}
permissionsDidChange = true;
#else
static constexpr auto writePerms = std::filesystem::perms::owner_write | std::filesystem::perms::group_write | std::filesystem::perms::others_write;
const auto stdStrPath = path.toStdWString();
const auto currentPermissions = std::filesystem::status(stdStrPath).permissions();
qCDebug(lcFileSystem()).nospace() << "current permissions path=" << path << " perms=" << Qt::showbase << Qt::oct << static_cast<int>(currentPermissions);
try
{
switch (permissions) {
case OCC::FileSystem::FolderPermissions::ReadOnly:
case OCC::FileSystem::FolderPermissions::ReadOnly: {
qCDebug(lcFileSystem()).nospace() << "ensuring folder is read only path=" << path;
if ((currentPermissions & writePerms) != std::filesystem::perms::none) {
qCDebug(lcFileSystem()).nospace() << "removing owner/group/others write permissions path=" << path;
std::filesystem::permissions(stdStrPath, writePerms, std::filesystem::perm_options::remove);
permissionsDidChange = true;
}
break;
case OCC::FileSystem::FolderPermissions::ReadWrite:
}
case OCC::FileSystem::FolderPermissions::ReadWrite: {
qCDebug(lcFileSystem()).nospace() << "ensuring folder is read/writable path=" << path;
if ((currentPermissions & std::filesystem::perms::others_write) != std::filesystem::perms::none) {
qCDebug(lcFileSystem()).nospace() << "removing others write permissions path=" << path;
std::filesystem::permissions(stdStrPath, std::filesystem::perms::others_write, std::filesystem::perm_options::remove);
permissionsDidChange = true;
}
if ((currentPermissions & std::filesystem::perms::owner_write) == std::filesystem::perms::none) {
qCDebug(lcFileSystem()).nospace() << "adding owner write permissions path=" << path;
std::filesystem::permissions(stdStrPath, std::filesystem::perms::owner_write, std::filesystem::perm_options::add);
permissionsDidChange = true;
}
break;
}
}
}
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcFileSystem()) << "exception when modifying folder permissions" << e.what() << "- path1:" << e.path1().c_str() << "- path2:" << e.path2().c_str();
@ -505,34 +541,16 @@ bool FileSystem::setFolderPermissions(const QString &path,
return false;
}
try
{
switch (permissions) {
case OCC::FileSystem::FolderPermissions::ReadOnly:
break;
case OCC::FileSystem::FolderPermissions::ReadWrite:
std::filesystem::permissions(stdStrPath, std::filesystem::perms::others_write, std::filesystem::perm_options::remove);
std::filesystem::permissions(stdStrPath, std::filesystem::perms::owner_write, std::filesystem::perm_options::add);
break;
}
}
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcFileSystem()) << "exception when modifying folder permissions" << e.what() << e.path1().c_str() << e.path2().c_str();
return false;
}
catch (const std::system_error &e)
{
qCWarning(lcFileSystem()) << "exception when modifying folder permissions" << e.what() << "- path:" << stdStrPath;
return false;
}
catch (...)
{
qCWarning(lcFileSystem()) << "exception when modifying folder permissions - path:" << stdStrPath;
return false;
if (permissionsDidChange) {
const auto newPermissions = std::filesystem::status(stdStrPath).permissions();
qCDebug(lcFileSystem()).nospace() << "updated permissions path=" << path << " perms=" << Qt::showbase << Qt::oct << static_cast<int>(newPermissions);
}
#endif
if (permissionsChanged) {
*permissionsChanged = permissionsDidChange;
}
return true;
}

3
src/libsync/filesystem.h
View File

@ -119,7 +119,8 @@ namespace FileSystem {
const std::function<void(const QString &path, bool isDir)> &onError = nullptr);
bool OWNCLOUDSYNC_EXPORT setFolderPermissions(const QString &path,
FileSystem::FolderPermissions permissions) noexcept;
FileSystem::FolderPermissions permissions,
bool *permissionsChanged = nullptr) noexcept;
bool OWNCLOUDSYNC_EXPORT isFolderReadOnly(const std::filesystem::path &path) noexcept;

2
test/CMakeLists.txt
View File

@ -99,6 +99,8 @@ nextcloud_add_test(SyncConflictsModel)
nextcloud_add_test(DateFieldBackend)
nextcloud_add_test(ClientStatusReporting)
nextcloud_add_test(FileSystem)
nextcloud_add_test(FolderStatusModel)
target_link_libraries(SecureFileDropTest PRIVATE Nextcloud::sync)

133
test/testfilesystem.cpp Normal file
View File

@ -0,0 +1,133 @@
/*
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: CC0-1.0
*
* This software is in the public domain, furnished "as is", without technical
* support, and with no warranty, express or implied, as to its usefulness for
* any purpose.
*/
#include <QtTest>
#include <QTemporaryDir>
#include "common/filesystembase.h"
#include "logger.h"
#include "libsync/filesystem.h"
using namespace OCC;
namespace std_fs = std::filesystem;
class TestFileSystem : public QObject
{
Q_OBJECT
private:
QTemporaryDir testDir;
private Q_SLOTS:
void initTestCase()
{
OCC::Logger::instance()->setLogFlush(true);
OCC::Logger::instance()->setLogDebug(true);
QStandardPaths::setTestModeEnabled(true);
QDir dir(testDir.path());
dir.mkdir("existingDirectory");
}
#ifndef Q_OS_WIN
void testSetFolderPermissionsExistingDirectory_data()
{
constexpr auto perms_0555 =
std_fs::perms::owner_read | std_fs::perms::owner_exec
| std_fs::perms::group_read | std_fs::perms::group_exec
| std_fs::perms::others_read | std_fs::perms::others_exec;
constexpr auto perms_0755 = perms_0555 | std_fs::perms::owner_write;
constexpr auto perms_0775 = perms_0755 | std_fs::perms::group_write;
QTest::addColumn<std_fs::perms>("originalPermissions");
QTest::addColumn<FileSystem::FolderPermissions>("folderPermissions");
QTest::addColumn<bool>("expectedResult");
QTest::addColumn<bool>("expectedPermissionsChanged");
QTest::addColumn<std_fs::perms>("expectedPermissions");
QTest::newRow("0777, readonly -> 0555, changed")
<< std_fs::perms::all
<< FileSystem::FolderPermissions::ReadOnly
<< true
<< true
<< perms_0555;
QTest::newRow("0555, readonly -> 0555, not changed")
<< perms_0555
<< FileSystem::FolderPermissions::ReadOnly
<< true
<< false
<< perms_0555;
QTest::newRow("0777, readwrite -> 0775, changed")
<< std_fs::perms::all
<< FileSystem::FolderPermissions::ReadWrite
<< true
<< true
<< perms_0775;
QTest::newRow("0775, readwrite -> 0775, not changed")
<< perms_0775
<< FileSystem::FolderPermissions::ReadWrite
<< true
<< false
<< perms_0775;
QTest::newRow("0755, readwrite -> 0755, not changed")
<< perms_0755
<< FileSystem::FolderPermissions::ReadWrite
<< true
<< false
<< perms_0755;
QTest::newRow("0555, readwrite -> 0755, changed")
<< perms_0555
<< FileSystem::FolderPermissions::ReadWrite
<< true
<< true
<< perms_0755;
}
void testSetFolderPermissionsExistingDirectory()
{
QFETCH(std_fs::perms, originalPermissions);
QFETCH(FileSystem::FolderPermissions, folderPermissions);
QFETCH(bool, expectedResult);
QFETCH(bool, expectedPermissionsChanged);
QFETCH(std_fs::perms, expectedPermissions);
bool permissionsDidChange = false;
QString fullPath = testDir.filePath("existingDirectory");
const auto stdStrPath = fullPath.toStdWString();
std_fs::permissions(stdStrPath, originalPermissions);
QCOMPARE(FileSystem::setFolderPermissions(fullPath, folderPermissions, &permissionsDidChange), expectedResult);
const auto newPermissions = std_fs::status(stdStrPath).permissions();
QCOMPARE(newPermissions, expectedPermissions);
QCOMPARE(permissionsDidChange, expectedPermissionsChanged);
}
void testSetFolderPermissionsNonexistentDirectory()
{
bool permissionsDidChange = false;
QString fullPath = testDir.filePath("nonexistentDirectory");
QCOMPARE(FileSystem::setFolderPermissions("nonexistentDirectory", FileSystem::FolderPermissions::ReadOnly, &permissionsDidChange), false);
QCOMPARE(permissionsDidChange, false);
}
#endif
};
QTEST_GUILESS_MAIN(TestFileSystem)
#include "testfilesystem.moc"