Skip to content

Commit

Permalink
fix failing automated tests and add new ones to make folders read-only
Browse files Browse the repository at this point in the history 
Signed-off-by: Matthieu Gallien <[email protected]>
  • Loading branch information
@mgallien
mgallien committed Feb 19, 2024
1 parent 406cea5 commit 3a8167e
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 11 deletions.
31 changes: 24 additions & 7 deletions src/common/filesystembase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#include <QFile>
#include <QCoreApplication>

#include <filesystem>

#include <sys/stat.h>
#include <sys/types.h>

Expand Down Expand Up @@ -146,12 +148,16 @@ bool FileSystem::rename(const QString &originFileName,
}
} else
#endif

try {
std::filesystem::rename(originFileName.toStdWString(), destinationFileName.toStdWString());
success = true;
}
catch (const std::filesystem::filesystem_error &e)
{
QFile orig(originFileName);
success = orig.rename(destinationFileName);
if (!success) {
error = orig.errorString();
}
qCWarning(lcFileSystem) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
success = false;
error = QString::fromUtf8(e.what());
}

if (!success) {
Expand Down Expand Up @@ -183,10 +189,21 @@ bool FileSystem::uncheckedRenameReplace(const QString &originFileName,
success = false;
}
if (success) {
success = orig.rename(destinationFileName);
try {
std::filesystem::rename(originFileName.toStdString(), destinationFileName.toStdString());
success = true;
}
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcFileSystem) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
success = false;
*errorString = QString::fromUtf8(e.what());
}
} else {
*errorString = orig.errorString();
}

if (!success) {
*errorString = orig.errorString();
qCWarning(lcFileSystem) << "Renaming temp file to final failed: " << *errorString;
return false;
}
Expand Down
11 changes: 8 additions & 3 deletions src/libsync/owncloudpropagator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1474,13 +1474,18 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
!_item->_remotePerm.hasPermission(RemotePermissions::CanMove) ||
!_item->_remotePerm.hasPermission(RemotePermissions::CanAddSubDirectories))) {
try {
std::filesystem::permissions(propagator()->fullLocalPath(_item->_file).toStdWString(), std::filesystem::perms::owner_write, std::filesystem::perm_options::add);
if (QFileInfo::exists(propagator()->fullLocalPath(_item->_file))) {
std::filesystem::permissions(propagator()->fullLocalPath(_item->_file).toStdWString(), std::filesystem::perms::owner_write, std::filesystem::perm_options::add);
}
if (QFileInfo::exists(propagator()->fullLocalPath(_item->_renameTarget))) {
std::filesystem::permissions(propagator()->fullLocalPath(_item->_renameTarget).toStdWString(), std::filesystem::perms::owner_write, std::filesystem::perm_options::add);
}
}
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcDirectory) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
_item->_status = SyncFileItem::NormalError;
_item->_errorString = tr("The folder %1 cannot be made read-only: %2").arg(_item->_file, e.what());
_item->_errorString = tr("The folder %1 cannot be made read-only: %2").arg(e.path1().c_str(), e.what());
}
}

Expand All @@ -1496,7 +1501,7 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status)
}
}
_state = Finished;
qCInfo(lcPropagator) << "PropagateDirectory::slotSubJobsFinished" << "emit finished" << status;
qCInfo(lcPropagator) << "PropagateDirectory::slotSubJobsFinished" << "emit finished" << status << _item->_file;
emit finished(status);
}

Expand Down
53 changes: 53 additions & 0 deletions src/libsync/propagatorjobs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,12 +303,65 @@ void PropagateLocalRename::start()
return;
}

auto targetParentFolderPath = std::filesystem::path{};
auto targetParentPermissions = std::filesystem::perms{};
try {
const auto newDirPath = std::filesystem::path{targetFile.toStdWString()};
Q_ASSERT(newDirPath.has_parent_path());
targetParentFolderPath = newDirPath.parent_path();
auto parentFolderStatus = std::filesystem::status(targetParentFolderPath);
targetParentPermissions = parentFolderStatus.permissions();
if ((targetParentPermissions & std::filesystem::perms::owner_write) != std::filesystem::perms::owner_write) {
std::filesystem::permissions(targetParentFolderPath, targetParentPermissions | std::filesystem::perms::owner_write, std::filesystem::perm_options::replace);
emit propagator()->touchedFile(QString::fromStdWString(targetParentFolderPath.wstring()));
}
}
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcPropagateLocalRename) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
}

auto originParentFolderPath = std::filesystem::path{};
auto originParentPermissions = std::filesystem::perms{};
try {
const auto newDirPath = std::filesystem::path{existingFile.toStdWString()};
Q_ASSERT(newDirPath.has_parent_path());
originParentFolderPath = newDirPath.parent_path();
auto parentFolderStatus = std::filesystem::status(originParentFolderPath);
originParentPermissions = parentFolderStatus.permissions();
if ((originParentPermissions & std::filesystem::perms::owner_write) != std::filesystem::perms::owner_write) {
std::filesystem::permissions(originParentFolderPath, originParentPermissions | std::filesystem::perms::owner_write, std::filesystem::perm_options::replace);
emit propagator()->touchedFile(QString::fromStdWString(originParentFolderPath.wstring()));
}
}
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcPropagateLocalRename) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
}

const auto restoreTargetPermissions = [this] (const auto &parentFolderPath, const auto &parentPermissions) {
try {
if ((parentPermissions & std::filesystem::perms::owner_write) != std::filesystem::perms::owner_write) {
std::filesystem::permissions(parentFolderPath, parentPermissions, std::filesystem::perm_options::replace);
emit propagator()->touchedFile(QString::fromStdWString(parentFolderPath.wstring()));
}
}
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcPropagateLocalRename) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
}
};

emit propagator()->touchedFile(existingFile);
emit propagator()->touchedFile(targetFile);
if (QString renameError; !FileSystem::rename(existingFile, targetFile, &renameError)) {
restoreTargetPermissions(targetParentFolderPath, targetParentPermissions);
restoreTargetPermissions(originParentFolderPath, originParentPermissions);
done(SyncFileItem::NormalError, renameError, ErrorCategory::GenericError);
return;
}
restoreTargetPermissions(targetParentFolderPath, targetParentPermissions);
restoreTargetPermissions(originParentFolderPath, originParentPermissions);
}

SyncJournalFileRecord oldRecord;
Expand Down
62 changes: 62 additions & 0 deletions test/testpermissions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,7 @@ private slots:

auto folderStatus = std::filesystem::status(static_cast<QString>(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString());
QVERIFY(folderStatus.permissions() & std::filesystem::perms::owner_read);
QVERIFY(!static_cast<bool>(folderStatus.permissions() & std::filesystem::perms::owner_write));

remote.find("testFolder")->permissions = RemotePermissions::fromServerString("WDNVRSM");

Expand All @@ -678,6 +679,67 @@ private slots:

folderStatus = std::filesystem::status(static_cast<QString>(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString());
QVERIFY(folderStatus.permissions() & std::filesystem::perms::owner_read);
QVERIFY(!static_cast<bool>(folderStatus.permissions() & std::filesystem::perms::owner_write));
}

void testChangePermissionsForFolderHierarchy()
{
FakeFolder fakeFolder{FileInfo{}};

auto &remote = fakeFolder.remoteModifier();

remote.mkdir("testFolder");

remote.find("testFolder")->permissions = RemotePermissions::fromServerString("M");

QVERIFY(fakeFolder.syncOnce());
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());

remote.mkdir("testFolder/subFolderReadWrite");
remote.mkdir("testFolder/subFolderReadOnly");

remote.find("testFolder/subFolderReadOnly")->permissions = RemotePermissions::fromServerString("m");

QVERIFY(fakeFolder.syncOnce());
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());

auto testFolderStatus = std::filesystem::status(static_cast<QString>(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString());
QVERIFY(testFolderStatus.permissions() & std::filesystem::perms::owner_read);
QVERIFY(!static_cast<bool>(testFolderStatus.permissions() & std::filesystem::perms::owner_write));
auto subFolderReadWriteStatus = std::filesystem::status(static_cast<QString>(fakeFolder.localPath() + QStringLiteral("/testFolder/subFolderReadWrite")).toStdWString());
QVERIFY(subFolderReadWriteStatus.permissions() & std::filesystem::perms::owner_read);
QVERIFY(subFolderReadWriteStatus.permissions() & std::filesystem::perms::owner_write);
auto subFolderReadOnlyStatus = std::filesystem::status(static_cast<QString>(fakeFolder.localPath() + QStringLiteral("/testFolder/subFolderReadOnly")).toStdWString());
QVERIFY(subFolderReadOnlyStatus.permissions() & std::filesystem::perms::owner_read);
QVERIFY(!static_cast<bool>(subFolderReadOnlyStatus.permissions() & std::filesystem::perms::owner_write));

remote.find("testFolder/subFolderReadOnly")->permissions = RemotePermissions::fromServerString("WDNVRSm");
remote.find("testFolder/subFolderReadWrite")->permissions = RemotePermissions::fromServerString("m");
remote.mkdir("testFolder/newSubFolder");
remote.create("testFolder/testFile", 12, '9');
remote.create("testFolder/testReadOnlyFile", 13, '8');
remote.find("testFolder/testReadOnlyFile")->permissions = RemotePermissions::fromServerString("m");

QVERIFY(fakeFolder.syncOnce());
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());

subFolderReadWriteStatus = std::filesystem::status(static_cast<QString>(fakeFolder.localPath() + QStringLiteral("/testFolder/subFolderReadWrite")).toStdWString());
QVERIFY(subFolderReadWriteStatus.permissions() & std::filesystem::perms::owner_read);
QVERIFY(!static_cast<bool>(subFolderReadWriteStatus.permissions() & std::filesystem::perms::owner_write));
subFolderReadOnlyStatus = std::filesystem::status(static_cast<QString>(fakeFolder.localPath() + QStringLiteral("/testFolder/subFolderReadOnly")).toStdWString());
QVERIFY(subFolderReadOnlyStatus.permissions() & std::filesystem::perms::owner_read);
QVERIFY(subFolderReadOnlyStatus.permissions() & std::filesystem::perms::owner_write);

remote.rename("testFolder/subFolderReadOnly", "testFolder/subFolderReadWriteNew");
remote.rename("testFolder/subFolderReadWrite", "testFolder/subFolderReadOnlyNew");
remote.rename("testFolder/testFile", "testFolder/testFileNew");
remote.rename("testFolder/testReadOnlyFile", "testFolder/testReadOnlyFileNew");

QVERIFY(fakeFolder.syncOnce());
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
testFolderStatus = std::filesystem::status(static_cast<QString>(fakeFolder.localPath() + QStringLiteral("/testFolder")).toStdWString());
QVERIFY(testFolderStatus.permissions() & std::filesystem::perms::owner_read);
QVERIFY(!static_cast<bool>(testFolderStatus.permissions() & std::filesystem::perms::owner_write));
}
};

Expand Down
7 changes: 6 additions & 1 deletion test/testsyncengine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ private slots:
Logger::instance()->setLogDebug(true);
}

void init()
{
QTextCodec::setCodecForLocale(QTextCodec::codecForName("UTF-8"));
}

void testFileDownload() {
FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
ItemCompletedSpy completeSpy(fakeFolder);
Expand Down Expand Up @@ -845,12 +850,12 @@ private slots:
QVERIFY(fakeFolder.syncOnce());
QVERIFY(fakeFolder.currentRemoteState().find("C/tößt"));

QTextCodec::setCodecForLocale(utf8Locale);
}
catch (const std::filesystem::filesystem_error &e)
{
qCritical() << e.what() << e.path1().c_str() << e.path2().c_str() << e.code().message().c_str();
}
QTextCodec::setCodecForLocale(utf8Locale);
#endif
}

Expand Down
6 changes: 6 additions & 0 deletions test/testsyncmove.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ class TestSyncMove : public QObject
Q_OBJECT

private slots:
void initTestCase()
{
Logger::instance()->setLogFlush(true);
Logger::instance()->setLogDebug(true);
}

void testMoveCustomRemoteRoot()
{
FileInfo subFolder(QStringLiteral("AS"), { { QStringLiteral("f1"), 4 } });
Expand Down

0 comments on commit 3a8167e

Please sign in to comment.