Skip to content

Commit

Permalink
ensure no crash due to uncatched exception from std c++ lib
Browse files Browse the repository at this point in the history 
Signed-off-by: Matthieu Gallien <[email protected]>
  • Loading branch information
@mgallien
mgallien committed Dec 18, 2024
1 parent 2d357f1 commit 92fa0df
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 17 deletions.
16 changes: 8 additions & 8 deletions src/common/filesystembase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -480,11 +480,11 @@ bool FileSystem::isWritable(const QString &filename, const QFileInfo &fileInfo)
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 (std::system_error e)
catch (const std::system_error &e)
{
qCWarning(lcFileSystem()) << filename << e.what();
}
Expand Down Expand Up @@ -514,11 +514,11 @@ bool FileSystem::isReadable(const QString &filename, const QFileInfo &fileInfo)
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 (std::system_error e)
catch (const std::system_error &e)
{
qCWarning(lcFileSystem()) << filename << e.what();
}
Expand Down
62 changes: 55 additions & 7 deletions src/libsync/filesystem.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,18 +332,31 @@ 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);
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 @@ bool FileSystem::setFolderPermissions(const QString &path,
}
#endif

try {
try
{
switch (permissions) {
case OCC::FileSystem::FolderPermissions::ReadOnly:
break;
Expand All @@ -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;
Expand All @@ -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()
Expand Down
24 changes: 24 additions & 0 deletions src/libsync/owncloudpropagator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
12 changes: 10 additions & 2 deletions src/libsync/propagatedownload.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}
Expand Down
52 changes: 52 additions & 0 deletions src/libsync/propagatorjobs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -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};
Expand Down
8 changes: 8 additions & 0 deletions test/testsyncengine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 92fa0df

Please sign in to comment.