From 4c77a7373fcf301ad1564ef5e314d8f87719bb9a Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Mon, 19 Aug 2024 13:33:36 +0200 Subject: [PATCH 1/2] add automated test for deletion of child items in read-only folders should ensure we are able to get files or dolders be deleted by sync engine within read-only folders would happen as the result of changes on server side being propagated locally or when using selective sync Signed-off-by: Matthieu Gallien --- test/testpermissions.cpp | 134 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/test/testpermissions.cpp b/test/testpermissions.cpp index 8d6c6297b4b19..1e2ab899b7ab6 100644 --- a/test/testpermissions.cpp +++ b/test/testpermissions.cpp @@ -746,6 +746,140 @@ private slots: QVERIFY(testFolderStatus.permissions() & std::filesystem::perms::owner_read); QVERIFY(!static_cast(testFolderStatus.permissions() & std::filesystem::perms::owner_write)); } + + void testDeleteChildItemsInReadOnlyFolder() + { + FakeFolder fakeFolder{FileInfo{}}; + + auto &remote = fakeFolder.remoteModifier(); + + remote.mkdir("readOnlyFolder"); + remote.mkdir("readOnlyFolder/test"); + remote.insert("readOnlyFolder/readOnlyFile.txt"); + + remote.find("readOnlyFolder")->permissions = RemotePermissions::fromServerString("M"); + remote.find("readOnlyFolder/test")->permissions = RemotePermissions::fromServerString("m"); + remote.find("readOnlyFolder/readOnlyFile.txt")->permissions = RemotePermissions::fromServerString("m"); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + remote.remove("readOnlyFolder/test"); + remote.remove("readOnlyFolder/readOnlyFile.txt"); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + const auto ensureReadOnlyItem = [&fakeFolder] (const auto path) -> bool { + const auto itemStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + path).toStdWString()); + return static_cast(itemStatus.permissions() & std::filesystem::perms::owner_read); + }; + QVERIFY(ensureReadOnlyItem("/readOnlyFolder")); + } + + void testRenameChildItemsInReadOnlyFolder() + { + FakeFolder fakeFolder{FileInfo{}}; + + auto &remote = fakeFolder.remoteModifier(); + + remote.mkdir("readOnlyFolder"); + remote.mkdir("readOnlyFolder/test"); + remote.insert("readOnlyFolder/readOnlyFile.txt"); + + remote.find("readOnlyFolder")->permissions = RemotePermissions::fromServerString("M"); + remote.find("readOnlyFolder/test")->permissions = RemotePermissions::fromServerString("m"); + remote.find("readOnlyFolder/readOnlyFile.txt")->permissions = RemotePermissions::fromServerString("m"); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + remote.rename("readOnlyFolder/test", "readOnlyFolder/testRename"); + remote.rename("readOnlyFolder/readOnlyFile.txt", "readOnlyFolder/readOnlyFileRename.txt"); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + const auto ensureReadOnlyItem = [&fakeFolder] (const auto path) -> bool { + const auto itemStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + path).toStdWString()); + return static_cast(itemStatus.permissions() & std::filesystem::perms::owner_read); + }; + QVERIFY(ensureReadOnlyItem("/readOnlyFolder")); + QVERIFY(ensureReadOnlyItem("/readOnlyFolder/testRename")); + QVERIFY(ensureReadOnlyItem("/readOnlyFolder/readOnlyFileRename.txt")); + } + + void testMoveChildItemsInReadOnlyFolder() + { + FakeFolder fakeFolder{FileInfo{}}; + + auto &remote = fakeFolder.remoteModifier(); + + remote.mkdir("readOnlyFolder"); + remote.mkdir("readOnlyFolder/child"); + remote.mkdir("readOnlyFolder/test"); + remote.insert("readOnlyFolder/readOnlyFile.txt"); + + remote.find("readOnlyFolder")->permissions = RemotePermissions::fromServerString("M"); + remote.find("readOnlyFolder/child")->permissions = RemotePermissions::fromServerString("m"); + remote.find("readOnlyFolder/test")->permissions = RemotePermissions::fromServerString("m"); + remote.find("readOnlyFolder/readOnlyFile.txt")->permissions = RemotePermissions::fromServerString("m"); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + remote.rename("readOnlyFolder/test", "readOnlyFolder/child/test"); + remote.rename("readOnlyFolder/readOnlyFile.txt", "readOnlyFolder/child/readOnlyFile.txt"); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + const auto ensureReadOnlyItem = [&fakeFolder] (const auto path) -> bool { + const auto itemStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + path).toStdWString()); + return static_cast(itemStatus.permissions() & std::filesystem::perms::owner_read); + }; + QVERIFY(ensureReadOnlyItem("/readOnlyFolder")); + QVERIFY(ensureReadOnlyItem("/readOnlyFolder/child")); + QVERIFY(ensureReadOnlyItem("/readOnlyFolder/child/test")); + QVERIFY(ensureReadOnlyItem("/readOnlyFolder/child/readOnlyFile.txt")); + } + + void testModifyChildItemsInReadOnlyFolder() + { + FakeFolder fakeFolder{FileInfo{}}; + + auto &remote = fakeFolder.remoteModifier(); + + remote.mkdir("readOnlyFolder"); + remote.mkdir("readOnlyFolder/test"); + remote.insert("readOnlyFolder/readOnlyFile.txt"); + + remote.find("readOnlyFolder")->permissions = RemotePermissions::fromServerString("M"); + remote.find("readOnlyFolder/test")->permissions = RemotePermissions::fromServerString("m"); + remote.find("readOnlyFolder/readOnlyFile.txt")->permissions = RemotePermissions::fromServerString("m"); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + remote.insert("readOnlyFolder/test/newFile.txt"); + remote.find("readOnlyFolder/test/newFile.txt")->permissions = RemotePermissions::fromServerString("m"); + remote.mkdir("readOnlyFolder/test/newFolder"); + remote.find("readOnlyFolder/test/newFolder")->permissions = RemotePermissions::fromServerString("m"); + remote.appendByte("readOnlyFolder/readOnlyFile.txt"); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + const auto ensureReadOnlyItem = [&fakeFolder] (const auto path) -> bool { + const auto itemStatus = std::filesystem::status(static_cast(fakeFolder.localPath() + path).toStdWString()); + return static_cast(itemStatus.permissions() & std::filesystem::perms::owner_read); + }; + QVERIFY(ensureReadOnlyItem("/readOnlyFolder")); + QVERIFY(ensureReadOnlyItem("/readOnlyFolder/test")); + QVERIFY(ensureReadOnlyItem("/readOnlyFolder/readOnlyFile.txt")); + QVERIFY(ensureReadOnlyItem("/readOnlyFolder/test/newFile.txt")); + QVERIFY(ensureReadOnlyItem("/readOnlyFolder/newFolder")); + } #endif }; From 20d60508f3cb34cca01066a946a32c4fe1f3af5a Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Mon, 19 Aug 2024 15:04:58 +0200 Subject: [PATCH 2/2] make folders read-write when needed when deleting local items if a folder is read-only, when deleting content inside this folder, it may be needed to make it temporary read-write do this as required by the automated tests Signed-off-by: Matthieu Gallien --- src/libsync/filesystem.cpp | 19 +++++++++++++++++++ src/libsync/filesystem.h | 13 +++++++++++++ src/libsync/propagatorjobs.cpp | 21 +++++++++++++++++---- 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 866eccdef12ef..0eb66c4f29fa2 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -480,6 +480,25 @@ bool FileSystem::isFolderReadOnly(const std::filesystem::path &path) noexcept return false; } } + +FileSystem::FilePermissionsRestore::FilePermissionsRestore(const QString &path, FolderPermissions temporaryPermissions) + : _path(path) +{ + const auto stdStrPath = _path.toStdWString(); + _initialPermissions = FileSystem::isFolderReadOnly(stdStrPath) ? OCC::FileSystem::FolderPermissions::ReadOnly : OCC::FileSystem::FolderPermissions::ReadWrite; + if (_initialPermissions != temporaryPermissions) { + _rollbackNeeded = true; + FileSystem::setFolderPermissions(_path, temporaryPermissions); + } +} + +FileSystem::FilePermissionsRestore::~FilePermissionsRestore() +{ + if (_rollbackNeeded) { + FileSystem::setFolderPermissions(_path, _initialPermissions); + } +} + #endif } // namespace OCC diff --git a/src/libsync/filesystem.h b/src/libsync/filesystem.h index 7028512fe84b3..dbb787cbb1616 100644 --- a/src/libsync/filesystem.h +++ b/src/libsync/filesystem.h @@ -43,6 +43,19 @@ class SyncJournal; * @brief This file contains file system helper */ namespace FileSystem { + class OWNCLOUDSYNC_EXPORT FilePermissionsRestore { + public: + explicit FilePermissionsRestore(const QString &path, + FileSystem::FolderPermissions temporaryPermissions); + + ~FilePermissionsRestore(); + + private: + QString _path; + FileSystem::FolderPermissions _initialPermissions; + bool _rollbackNeeded = false; + }; + struct OWNCLOUDSYNC_EXPORT FileLockingInfo { enum class Type { Unset = -1, Locked, Unlocked }; QString path; diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 4b18a944c7f61..37506b6607c54 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -63,6 +63,9 @@ bool PropagateLocalRemove::removeRecursively(const QString &path) QStringList errors; QList> deleted; #if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 + const auto fileInfo = QFileInfo{absolute}; + const auto parentFolderPath = fileInfo.dir().absolutePath(); + const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; FileSystem::setFolderPermissions(absolute, FileSystem::FolderPermissions::ReadWrite); #endif bool success = FileSystem::removeRecursively( @@ -126,10 +129,18 @@ void PropagateLocalRemove::start() return; } } else { - if (FileSystem::fileExists(filename) - && !FileSystem::remove(filename, &removeError)) { - done(SyncFileItem::NormalError, removeError, ErrorCategory::GenericError); - return; + if (FileSystem::fileExists(filename)) { +#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 + const auto fileInfo = QFileInfo{filename}; + const auto parentFolderPath = fileInfo.dir().absolutePath(); + + const auto parentPermissionsHandler = FileSystem::FilePermissionsRestore{parentFolderPath, FileSystem::FolderPermissions::ReadWrite}; +#endif + + if (!FileSystem::remove(filename, &removeError)) { + done(SyncFileItem::NormalError, removeError, ErrorCategory::GenericError); + return; + } } } } @@ -369,6 +380,8 @@ void PropagateLocalRename::start() }; #endif + const auto folderPermissionsHandler = FileSystem::FilePermissionsRestore{existingFile, FileSystem::FolderPermissions::ReadWrite}; + emit propagator()->touchedFile(existingFile); emit propagator()->touchedFile(targetFile); if (QString renameError; !FileSystem::rename(existingFile, targetFile, &renameError)) {