From 92fa0df182e10a4d0d094be72c87f8a4896c9c0b Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Wed, 18 Dec 2024 12:36:05 +0100 Subject: [PATCH] ensure no crash due to uncatched exception from std c++ lib Signed-off-by: Matthieu Gallien --- src/common/filesystembase.cpp | 16 ++++---- src/libsync/filesystem.cpp | 62 ++++++++++++++++++++++++++---- src/libsync/owncloudpropagator.cpp | 24 ++++++++++++ src/libsync/propagatedownload.cpp | 12 +++++- src/libsync/propagatorjobs.cpp | 52 +++++++++++++++++++++++++ test/testsyncengine.cpp | 8 ++++ 6 files changed, 157 insertions(+), 17 deletions(-) diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index af9e19cb65bd1..cf806fd8e8376 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -127,11 +127,11 @@ void FileSystem::setFileReadOnly(const QString &filename, bool readonly) 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 (std::system_error e) + catch (const std::system_error &e) { qCWarning(lcFileSystem()) << filename << e.what(); } @@ -180,11 +180,11 @@ bool FileSystem::setFileReadOnlyWeak(const QString &filename, bool readonly) 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 (std::system_error e) + catch (const std::system_error &e) { qCWarning(lcFileSystem()) << filename << e.what(); } @@ -480,11 +480,11 @@ bool FileSystem::isWritable(const QString &filename, const QFileInfo &fileInfo) const auto permissions = filePermissionsWin(filename); return static_cast((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 (std::system_error e) + catch (const std::system_error &e) { qCWarning(lcFileSystem()) << filename << e.what(); } @@ -514,11 +514,11 @@ bool FileSystem::isReadable(const QString &filename, const QFileInfo &fileInfo) const auto permissions = filePermissionsWin(filename); return static_cast((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 (std::system_error e) + catch (const std::system_error &e) { qCWarning(lcFileSystem()) << filename << e.what(); } diff --git a/src/libsync/filesystem.cpp b/src/libsync/filesystem.cpp index 9dbdde6a7474c..2f288b85fe5bd 100644 --- a/src/libsync/filesystem.cpp +++ b/src/libsync/filesystem.cpp @@ -332,7 +332,8 @@ bool FileSystem::setFolderPermissions(const QString &path, { 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); @@ -340,10 +341,22 @@ bool FileSystem::setFolderPermissions(const QString &path, 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; @@ -463,7 +476,8 @@ bool FileSystem::setFolderPermissions(const QString &path, } #endif - try { + try + { switch (permissions) { case OCC::FileSystem::FolderPermissions::ReadOnly: break; @@ -472,17 +486,30 @@ bool FileSystem::setFolderPermissions(const QString &path, 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; @@ -492,21 +519,42 @@ bool FileSystem::isFolderReadOnly(const std::filesystem::path &path) noexcept 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() diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index fc166ecce8e90..5c247e2cc246d 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -1472,6 +1472,18 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status) _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) @@ -1494,6 +1506,18 @@ void PropagateDirectory::slotSubJobsFinished(SyncFileItem::Status status) _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) { diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 3bcfd237316dc..39108e4e113d4 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -690,6 +690,14 @@ void PropagateDownloadFile::startDownload() { 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); @@ -1219,11 +1227,11 @@ void PropagateDownloadFile::downloadFinished() 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 (std::system_error e) + catch (const std::system_error &e) { qCWarning(lcPropagateDownload()) << _item->_instruction << _item->_file << e.what(); } diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 746e29e79eac3..6d1bb1b6fddcf 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -217,6 +217,14 @@ void PropagateLocalMkdir::startLocalMkdir() { 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); @@ -239,6 +247,18 @@ void PropagateLocalMkdir::startLocalMkdir() 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 { @@ -251,6 +271,14 @@ void PropagateLocalMkdir::startLocalMkdir() { 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, @@ -349,6 +377,14 @@ void PropagateLocalRename::start() { 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; @@ -366,6 +402,14 @@ void PropagateLocalRename::start() { 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 { @@ -376,6 +420,14 @@ void PropagateLocalRename::start() { 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}; diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index c745ddcad4ffe..f10aae80036ab 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -860,6 +860,14 @@ private slots: { 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 }