Skip to content
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.

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

[stable-3.15] Bugfix/catch exceptions to prevent crash #7705

Merged
merged 2 commits into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 36 additions & 4 deletions src/common/filesystembase.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/common/filesystembase.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/common/filesystembase.cpp

File src/common/filesystembase.cpp does not conform to Custom style guidelines. (lines 130, 134, 137, 138, 183, 187, 190, 191, 483, 487, 490, 491, 517, 521, 524, 525)
* Copyright (C) by Daniel Molkentin <[email protected]>
*
* This library is free software; you can redistribute it and/or
Expand Down Expand Up @@ -127,10 +127,18 @@
std::filesystem::permissions(filename.toStdString(), defaultWritePermissions, std::filesystem::perm_options::add);
}
}
catch (std::filesystem::filesystem_error e)
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;
}
return;
}
#endif
Expand Down Expand Up @@ -172,10 +180,18 @@
setFileReadOnly(filename, readonly);
return true;
}
catch (std::filesystem::filesystem_error e)
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;
}
return false;
}
#endif
Expand Down Expand Up @@ -464,10 +480,18 @@
const auto permissions = filePermissionsWin(filename);
return static_cast<bool>((permissions & std::filesystem::perms::owner_write));
}
catch (std::filesystem::filesystem_error e)
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcFileSystem()) << filename << e.what();
}
catch (const std::system_error &e)
{
qCWarning(lcFileSystem()) << filename << e.what();
}
catch (...)
{
qCWarning(lcFileSystem()) << filename;
}
return false;
}
#endif
Expand All @@ -490,10 +514,18 @@
const auto permissions = filePermissionsWin(filename);
return static_cast<bool>((permissions & std::filesystem::perms::owner_read));
}
catch (std::filesystem::filesystem_error e)
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcFileSystem()) << filename << e.what();
}
catch (const std::system_error &e)
{
qCWarning(lcFileSystem()) << filename << e.what();
}
catch (...)
{
qCWarning(lcFileSystem()) << filename;
}
return false;
}
#endif
Expand Down
62 changes: 55 additions & 7 deletions src/libsync/filesystem.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/filesystem.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/filesystem.cpp

File src/libsync/filesystem.cpp does not conform to Custom style guidelines. (lines 335, 344, 345, 350, 354, 355, 479, 489, 490, 495, 499, 500, 511, 522, 526, 527, 537, 545, 546, 550, 553, 554)
* Copyright (C) by Daniel Molkentin <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand Down Expand Up @@ -332,18 +332,31 @@
{
static constexpr auto writePerms = std::filesystem::perms::owner_write | std::filesystem::perms::group_write | std::filesystem::perms::others_write;
const auto stdStrPath = path.toStdWString();
try {
try
{
switch (permissions) {
case OCC::FileSystem::FolderPermissions::ReadOnly:
std::filesystem::permissions(stdStrPath, writePerms, std::filesystem::perm_options::remove);
break;
case OCC::FileSystem::FolderPermissions::ReadWrite:
break;
}
} catch (const std::filesystem::filesystem_error &e) {
}
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();
return false;
}
catch (...)
{
qCWarning(lcFileSystem()) << "exception when modifying folder permissions";
return false;
}

#ifdef Q_OS_WIN
SECURITY_INFORMATION info = DACL_SECURITY_INFORMATION;
Expand Down Expand Up @@ -463,7 +476,8 @@
}
#endif

try {
try
{
switch (permissions) {
case OCC::FileSystem::FolderPermissions::ReadOnly:
break;
Expand All @@ -472,17 +486,30 @@
std::filesystem::permissions(stdStrPath, std::filesystem::perms::owner_write, std::filesystem::perm_options::add);
break;
}
} catch (const std::filesystem::filesystem_error &e) {
}
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();
return false;
}
catch (...)
{
qCWarning(lcFileSystem()) << "exception when modifying folder permissions";
return false;
}

return true;
}

bool FileSystem::isFolderReadOnly(const std::filesystem::path &path) noexcept
{
try {
try
{
const auto folderStatus = std::filesystem::status(path);
const auto folderPermissions = folderStatus.permissions();
return (folderPermissions & std::filesystem::perms::owner_write) != std::filesystem::perms::owner_write;
Expand All @@ -492,21 +519,42 @@
qCWarning(lcFileSystem()) << "exception when checking folder permissions" << e.what() << e.path1().c_str() << e.path2().c_str();
return false;
}
catch (const std::system_error &e)
{
qCWarning(lcFileSystem()) << "exception when checking folder permissions" << e.what();
return false;
}
catch (...)
{
qCWarning(lcFileSystem()) << "exception when checking folder permissions";
return false;
}
}

FileSystem::FilePermissionsRestore::FilePermissionsRestore(const QString &path, FolderPermissions temporaryPermissions)
: _path(path)
{
try {
try
{
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);
}
} catch (const std::filesystem::filesystem_error &e) {
}
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcFileSystem()) << "exception when modifying folder permissions" << e.what() << e.path1().c_str() << e.path2().c_str();
}
catch (const std::system_error &e)
{
qCWarning(lcFileSystem()) << "exception when modifying folder permissions" << e.what();
}
catch (...)
{
qCWarning(lcFileSystem()) << "exception when modifying folder permissions";
}
}

FileSystem::FilePermissionsRestore::~FilePermissionsRestore()
Expand Down
24 changes: 24 additions & 0 deletions src/libsync/owncloudpropagator.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/owncloudpropagator.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/owncloudpropagator.cpp

File src/libsync/owncloudpropagator.cpp does not conform to Custom style guidelines. (lines 1475, 1480, 1481, 1509, 1514, 1515)
* Copyright (C) by Olivier Goffart <[email protected]>
* Copyright (C) by Klaas Freitag <[email protected]>
*
Expand Down Expand Up @@ -1472,6 +1472,18 @@
_item->_status = SyncFileItem::NormalError;
_item->_errorString = tr("The folder %1 cannot be made read-only: %2").arg(_item->_file, e.what());
}
catch (const std::system_error &e)
{
qCWarning(lcDirectory) << "exception when checking parent folder access rights" << e.what();
_item->_status = SyncFileItem::NormalError;
_item->_errorString = tr("The folder %1 cannot be made read-only: %2").arg(_item->_file, e.what());
}
catch (...)
{
qCWarning(lcDirectory) << "exception when checking parent folder access rights";
_item->_status = SyncFileItem::NormalError;
_item->_errorString = tr("The folder %1 cannot be made read-only: %2").arg(_item->_file, tr("unknown exception"));
}
} else {
try {
const auto permissionsChangeHelper = [] (const auto fileName)
Expand All @@ -1494,6 +1506,18 @@
_item->_status = SyncFileItem::NormalError;
_item->_errorString = tr("The folder %1 cannot be made read-only: %2").arg(e.path1().c_str(), e.what());
}
catch (const std::system_error &e)
{
qCWarning(lcDirectory) << "exception when checking parent folder access rights" << e.what();
_item->_status = SyncFileItem::NormalError;
_item->_errorString = tr("The folder %1 cannot be made read-only: %2").arg("", e.what());
}
catch (...)
{
qCWarning(lcDirectory) << "exception when checking parent folder access rights";
_item->_status = SyncFileItem::NormalError;
_item->_errorString = tr("The folder %1 cannot be made read-only: %2").arg("", tr("unknown exception"));
}
}
#endif
if (!_item->_isAnyCaseClashChild && !_item->_isAnyInvalidCharChild) {
Expand Down
18 changes: 17 additions & 1 deletion src/libsync/propagatedownload.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/propagatedownload.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/propagatedownload.cpp

File src/libsync/propagatedownload.cpp does not conform to Custom style guidelines. (lines 693, 696, 697, 1230, 1234, 1237, 1238)
* Copyright (C) by Olivier Goffart <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
Expand All @@ -12,7 +12,7 @@
* for more details.
*/

#include "config.h"

Check failure on line 15 in src/libsync/propagatedownload.cpp

View workflow job for this annotation

GitHub Actions / build

src/libsync/propagatedownload.cpp:15:10 [clang-diagnostic-error]

'config.h' file not found
#include "owncloudpropagator_p.h"
#include "propagatedownload.h"
#include "networkjobs.h"
Expand Down Expand Up @@ -690,6 +690,14 @@
{
qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
}
catch (const std::system_error &e)
{
qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights" << e.what();
}
catch (...)
{
qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights";
}

if (FileSystem::isFolderReadOnly(_parentPath)) {
FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadWrite);
Expand Down Expand Up @@ -1219,10 +1227,18 @@
FileSystem::setFilePermissionsWin(_tmpFile.fileName(), existingPermissions);
}
}
catch (std::filesystem::filesystem_error e)
catch (const std::filesystem::filesystem_error &e)
{
qCWarning(lcPropagateDownload()) << _item->_instruction << _item->_file << e.what();
}
catch (const std::system_error &e)
{
qCWarning(lcPropagateDownload()) << _item->_instruction << _item->_file << e.what();
}
catch (...)
{
qCWarning(lcPropagateDownload()) << _item->_instruction << _item->_file;
}
#else
if (existingFile.permissions() != _tmpFile.permissions()) {
_tmpFile.setPermissions(existingFile.permissions());
Expand Down
52 changes: 52 additions & 0 deletions src/libsync/propagatorjobs.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*

Check notice on line 1 in src/libsync/propagatorjobs.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on src/libsync/propagatorjobs.cpp

File src/libsync/propagatorjobs.cpp does not conform to Custom style guidelines. (lines 220, 223, 224, 250, 255, 256, 259, 274, 277, 278, 380, 383, 384, 405, 408, 409, 423, 426, 427)
* Copyright (C) by Olivier Goffart <[email protected]>
* Copyright (C) by Klaas Freitag <[email protected]>
*
Expand Down Expand Up @@ -217,6 +217,14 @@
{
qCWarning(lcPropagateLocalMkdir) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
}
catch (const std::system_error &e)
{
qCWarning(lcPropagateLocalMkdir) << "exception when checking parent folder access rights" << e.what();
}
catch (...)
{
qCWarning(lcPropagateLocalMkdir) << "exception when checking parent folder access rights";
}
#endif

emit propagator()->touchedFile(newDirStr);
Expand All @@ -239,6 +247,18 @@
done(SyncFileItem::NormalError, tr("The folder %1 cannot be made read-only: %2").arg(_item->_file, e.what()), ErrorCategory::GenericError);
return;
}
catch (const std::system_error &e)
{
qCWarning(lcPropagateLocalMkdir) << "exception when checking parent folder access rights" << e.what();
done(SyncFileItem::NormalError, tr("The folder %1 cannot be made read-only: %2").arg(_item->_file, e.what()), ErrorCategory::GenericError);
return;
}
catch (...)
{
qCWarning(lcPropagateLocalMkdir) << "exception when checking parent folder access rights";
done(SyncFileItem::NormalError, tr("The folder %1 cannot be made read-only: %2").arg(_item->_file, tr("unknown exception")), ErrorCategory::GenericError);
return;
}
}

try {
Expand All @@ -251,6 +271,14 @@
{
qCWarning(lcPropagateLocalMkdir) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
}
catch (const std::system_error &e)
{
qCWarning(lcPropagateLocalMkdir) << "exception when checking parent folder access rights" << e.what();
}
catch (...)
{
qCWarning(lcPropagateLocalMkdir) << "exception when checking parent folder access rights";
}
#endif

// Insert the directory into the database. The correct etag will be set later,
Expand Down Expand Up @@ -349,6 +377,14 @@
{
qCWarning(lcPropagateLocalRename) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
}
catch (const std::system_error &e)
{
qCWarning(lcPropagateLocalRename) << "exception when checking parent folder access rights" << e.what();
}
catch (...)
{
qCWarning(lcPropagateLocalRename) << "exception when checking parent folder access rights";
}

auto originParentFolderPath = std::filesystem::path{};
auto originParentFolderWasReadOnly = false;
Expand All @@ -366,6 +402,14 @@
{
qCWarning(lcPropagateLocalRename) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
}
catch (const std::system_error &e)
{
qCWarning(lcPropagateLocalRename) << "exception when checking parent folder access rights" << e.what();
}
catch (...)
{
qCWarning(lcPropagateLocalRename) << "exception when checking parent folder access rights";
}

const auto restoreTargetPermissions = [this] (const auto &parentFolderPath) {
try {
Expand All @@ -376,6 +420,14 @@
{
qCWarning(lcPropagateLocalRename) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str();
}
catch (const std::system_error &e)
{
qCWarning(lcPropagateLocalRename) << "exception when checking parent folder access rights" << e.what();
}
catch (...)
{
qCWarning(lcPropagateLocalRename) << "exception when checking parent folder access rights";
}
};

const auto folderPermissionsHandler = FileSystem::FilePermissionsRestore{existingFile, FileSystem::FolderPermissions::ReadWrite};
Expand Down
8 changes: 8 additions & 0 deletions test/testsyncengine.cpp
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/*

Check notice on line 1 in test/testsyncengine.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on test/testsyncengine.cpp

File test/testsyncengine.cpp does not conform to Custom style guidelines. (lines 863, 866, 867)
* 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>

Check failure on line 8 in test/testsyncengine.cpp

View workflow job for this annotation

GitHub Actions / build

test/testsyncengine.cpp:8:10 [clang-diagnostic-error]

'QtTest' file not found
#include <QTextCodec>

#include "syncenginetestutils.h"
Expand Down Expand Up @@ -860,6 +860,14 @@
{
qCritical() << e.what() << e.path1().c_str() << e.path2().c_str() << e.code().message().c_str();
}
catch (const std::system_error &e)
{
qCritical() << e.what() << e.code().message().c_str();
}
catch (...)
{
qCritical() << "exception unknown";
}
QTextCodec::setCodecForLocale(utf8Locale);
#endif
}
Expand Down
Loading