From 79ca2e145f0d67b814bbc0a49b33f12ddc95116b Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Wed, 14 Jun 2023 13:38:19 +0800 Subject: [PATCH] Don't read unlimited data from files It now guards against reading infinite files such as `/dev/zero`. And most readings are bound with a (lax) limit. As a side effect, more checking are done when reading a file and overall the reading procedure is more robust. PR #19095. --- .../bittorrent/bencoderesumedatastorage.cpp | 25 ++-- src/base/bittorrent/dbresumedatastorage.cpp | 1 - src/base/bittorrent/sessionimpl.cpp | 20 +-- src/base/bittorrent/torrentimpl.cpp | 1 - src/base/bittorrent/torrentinfo.cpp | 20 +-- src/base/rss/feed_serializer.cpp | 29 ++--- src/base/rss/rss_autodownloader.cpp | 27 ++-- src/base/rss/rss_session.cpp | 31 ++--- src/base/search/searchpluginmanager.cpp | 7 +- src/base/torrentfileswatcher.cpp | 51 +++++--- src/base/utils/io.cpp | 32 +++++ src/base/utils/io.h | 19 +++ src/gui/aboutdialog.cpp | 24 ++-- src/gui/optionsdialog.cpp | 37 ++---- src/gui/previewselectdialog.cpp | 1 - src/gui/rss/automatedrssdownloader.cpp | 22 ++-- src/gui/uithemedialog.cpp | 1 - src/gui/uithemesource.cpp | 50 +++++--- src/gui/utils.cpp | 6 +- src/webui/webapplication.cpp | 37 +++--- src/webui/webui.cpp | 9 +- test/CMakeLists.txt | 1 + test/testdata/size10.txt | 1 + test/testutilsio.cpp | 115 ++++++++++++++++++ 24 files changed, 369 insertions(+), 198 deletions(-) create mode 100644 test/testdata/size10.txt create mode 100644 test/testutilsio.cpp diff --git a/src/base/bittorrent/bencoderesumedatastorage.cpp b/src/base/bittorrent/bencoderesumedatastorage.cpp index a3c0614f4..6c449cbf1 100644 --- a/src/base/bittorrent/bencoderesumedatastorage.cpp +++ b/src/base/bittorrent/bencoderesumedatastorage.cpp @@ -36,6 +36,7 @@ #include #include +#include #include #include @@ -133,17 +134,19 @@ BitTorrent::LoadResumeDataResult BitTorrent::BencodeResumeDataStorage::load(cons const Path fastresumePath = path() / Path(idString + u".fastresume"); const Path torrentFilePath = path() / Path(idString + u".torrent"); - QFile resumeDataFile {fastresumePath.data()}; - if (!resumeDataFile.open(QIODevice::ReadOnly)) - return nonstd::make_unexpected(tr("Cannot read file %1: %2").arg(fastresumePath.toString(), resumeDataFile.errorString())); + const auto resumeDataReadResult = Utils::IO::readFile(fastresumePath, MAX_TORRENT_SIZE); + if (!resumeDataReadResult) + return nonstd::make_unexpected(resumeDataReadResult.error().message); - QFile metadataFile {torrentFilePath.data()}; - if (metadataFile.exists() && !metadataFile.open(QIODevice::ReadOnly)) - return nonstd::make_unexpected(tr("Cannot read file %1: %2").arg(torrentFilePath.toString(), metadataFile.errorString())); - - const QByteArray data = resumeDataFile.readAll(); - const QByteArray metadata = (metadataFile.isOpen() ? metadataFile.readAll() : ""); + const auto metadataReadResult = Utils::IO::readFile(torrentFilePath, MAX_TORRENT_SIZE); + if (!metadataReadResult) + { + if (metadataReadResult.error().status != Utils::IO::ReadError::NotExist) + return nonstd::make_unexpected(metadataReadResult.error().message); + } + const QByteArray data = resumeDataReadResult.value(); + const QByteArray metadata = metadataReadResult.value_or(QByteArray()); return loadTorrentResumeData(data, metadata); } @@ -161,6 +164,8 @@ void BitTorrent::BencodeResumeDataStorage::doLoadAll() const void BitTorrent::BencodeResumeDataStorage::loadQueue(const Path &queueFilename) { + const int lineMaxLength = 48; + QFile queueFile {queueFilename.data()}; if (!queueFile.exists()) return; @@ -175,7 +180,7 @@ void BitTorrent::BencodeResumeDataStorage::loadQueue(const Path &queueFilename) int start = 0; while (true) { - const auto line = QString::fromLatin1(queueFile.readLine().trimmed()); + const auto line = QString::fromLatin1(queueFile.readLine(lineMaxLength).trimmed()); if (line.isEmpty()) break; diff --git a/src/base/bittorrent/dbresumedatastorage.cpp b/src/base/bittorrent/dbresumedatastorage.cpp index 7738d6b9b..8b3f61714 100644 --- a/src/base/bittorrent/dbresumedatastorage.cpp +++ b/src/base/bittorrent/dbresumedatastorage.cpp @@ -41,7 +41,6 @@ #include #include -#include #include #include #include diff --git a/src/base/bittorrent/sessionimpl.cpp b/src/base/bittorrent/sessionimpl.cpp index 3a10a9399..0542b40fc 100644 --- a/src/base/bittorrent/sessionimpl.cpp +++ b/src/base/bittorrent/sessionimpl.cpp @@ -60,7 +60,6 @@ #include #include -#include #include #include #include @@ -5101,8 +5100,8 @@ void SessionImpl::loadCategories() { m_categories.clear(); - QFile confFile {(specialFolderLocation(SpecialFolder::Config) / CATEGORIES_FILE_NAME).data()}; - if (!confFile.exists()) + const Path path = specialFolderLocation(SpecialFolder::Config) / CATEGORIES_FILE_NAME; + if (!path.exists()) { // TODO: Remove the following upgrade code in v4.5 // == BEGIN UPGRADE CODE == @@ -5113,26 +5112,27 @@ void SessionImpl::loadCategories() // return; } - if (!confFile.open(QFile::ReadOnly)) + const int fileMaxSize = 1024 * 1024; + const auto readResult = Utils::IO::readFile(path, fileMaxSize); + if (!readResult) { - LogMsg(tr("Failed to load Categories. File: \"%1\". Error: \"%2\"") - .arg(confFile.fileName(), confFile.errorString()), Log::CRITICAL); + LogMsg(tr("Failed to load Categories. %1").arg(readResult.error().message), Log::WARNING); return; } QJsonParseError jsonError; - const QJsonDocument jsonDoc = QJsonDocument::fromJson(confFile.readAll(), &jsonError); + const QJsonDocument jsonDoc = QJsonDocument::fromJson(readResult.value(), &jsonError); if (jsonError.error != QJsonParseError::NoError) { LogMsg(tr("Failed to parse Categories configuration. File: \"%1\". Error: \"%2\"") - .arg(confFile.fileName(), jsonError.errorString()), Log::WARNING); + .arg(path.toString(), jsonError.errorString()), Log::WARNING); return; } if (!jsonDoc.isObject()) { - LogMsg(tr("Failed to load Categories configuration. File: \"%1\". Reason: invalid data format") - .arg(confFile.fileName()), Log::WARNING); + LogMsg(tr("Failed to load Categories configuration. File: \"%1\". Error: \"Invalid data format\"") + .arg(path.toString()), Log::WARNING); return; } diff --git a/src/base/bittorrent/torrentimpl.cpp b/src/base/bittorrent/torrentimpl.cpp index 4abc49c6a..9462c54db 100644 --- a/src/base/bittorrent/torrentimpl.cpp +++ b/src/base/bittorrent/torrentimpl.cpp @@ -46,7 +46,6 @@ #include #include -#include #include #include #include diff --git a/src/base/bittorrent/torrentinfo.cpp b/src/base/bittorrent/torrentinfo.cpp index 01080e28b..42d34e688 100644 --- a/src/base/bittorrent/torrentinfo.cpp +++ b/src/base/bittorrent/torrentinfo.cpp @@ -103,28 +103,20 @@ nonstd::expected TorrentInfo::load(const QByteArray &data) nonstd::expected TorrentInfo::loadFromFile(const Path &path) noexcept { - QFile file {path.data()}; - if (!file.open(QIODevice::ReadOnly)) - return nonstd::make_unexpected(file.errorString()); - - if (file.size() > MAX_TORRENT_SIZE) - return nonstd::make_unexpected(tr("File size exceeds max limit %1").arg(Utils::Misc::friendlyUnit(MAX_TORRENT_SIZE))); - QByteArray data; try { - data = file.readAll(); + const auto readResult = Utils::IO::readFile(path, MAX_TORRENT_SIZE); + if (!readResult) + return nonstd::make_unexpected(readResult.error().message); + data = readResult.value(); } catch (const std::bad_alloc &e) { - return nonstd::make_unexpected(tr("Torrent file read error: %1").arg(QString::fromLocal8Bit(e.what()))); + return nonstd::make_unexpected(tr("Failed to allocate memory when reading file. File: \"%1\". Error: \"%2\"") + .arg(path.toString(), QString::fromLocal8Bit(e.what()))); } - if (data.size() != file.size()) - return nonstd::make_unexpected(tr("Torrent file read error: size mismatch")); - - file.close(); - return load(data); } diff --git a/src/base/rss/feed_serializer.cpp b/src/base/rss/feed_serializer.cpp index d22fae28d..062360ba7 100644 --- a/src/base/rss/feed_serializer.cpp +++ b/src/base/rss/feed_serializer.cpp @@ -31,7 +31,6 @@ #include "feed_serializer.h" -#include #include #include #include @@ -46,23 +45,21 @@ const int ARTICLEDATALIST_TYPEID = qRegisterMetaType>(); void RSS::Private::FeedSerializer::load(const Path &dataFileName, const QString &url) { - QFile file {dataFileName.data()}; - - if (!file.exists()) - { - emit loadingFinished({}); - } - else if (file.open(QFile::ReadOnly)) - { - emit loadingFinished(loadArticles(file.readAll(), url)); - file.close(); - } - else + const int fileMaxSize = 10 * 1024 * 1024; + const auto readResult = Utils::IO::readFile(dataFileName, fileMaxSize); + if (!readResult) { - LogMsg(tr("Couldn't read RSS Session data from %1. Error: %2") - .arg(dataFileName.toString(), file.errorString()) - , Log::WARNING); + if (readResult.error().status == Utils::IO::ReadError::NotExist) + { + emit loadingFinished({}); + return; + } + + LogMsg(tr("Failed to read RSS session data. %1").arg(readResult.error().message), Log::WARNING); + return; } + + emit loadingFinished(loadArticles(readResult.value(), url)); } void RSS::Private::FeedSerializer::store(const Path &dataFileName, const QVector &articlesData) diff --git a/src/base/rss/rss_autodownloader.cpp b/src/base/rss/rss_autodownloader.cpp index bf7072fd8..33131c7e2 100644 --- a/src/base/rss/rss_autodownloader.cpp +++ b/src/base/rss/rss_autodownloader.cpp @@ -48,6 +48,7 @@ #include "../logger.h" #include "../profile.h" #include "../utils/fs.h" +#include "../utils/io.h" #include "rss_article.h" #include "rss_autodownloadrule.h" #include "rss_feed.h" @@ -493,21 +494,21 @@ void AutoDownloader::processJob(const QSharedPointer &job) void AutoDownloader::load() { - QFile rulesFile {(m_fileStorage->storageDir() / Path(RULES_FILE_NAME)).data()}; - - if (!rulesFile.exists()) - { - loadRulesLegacy(); - } - else if (rulesFile.open(QFile::ReadOnly)) - { - loadRules(rulesFile.readAll()); - } - else + const qint64 maxFileSize = 10 * 1024 * 1024; + const auto readResult = Utils::IO::readFile((m_fileStorage->storageDir() / Path(RULES_FILE_NAME)), maxFileSize); + if (!readResult) { - LogMsg(tr("Couldn't read RSS AutoDownloader rules from %1. Error: %2") - .arg(rulesFile.fileName(), rulesFile.errorString()), Log::CRITICAL); + if (readResult.error().status == Utils::IO::ReadError::NotExist) + { + loadRulesLegacy(); + return; + } + + LogMsg((tr("Failed to read RSS AutoDownloader rules. %1").arg(readResult.error().message)), Log::WARNING); + return; } + + loadRules(readResult.value()); } void AutoDownloader::loadRules(const QByteArray &data) diff --git a/src/base/rss/rss_session.cpp b/src/base/rss/rss_session.cpp index ef01e5b98..a4d13220a 100644 --- a/src/base/rss/rss_session.cpp +++ b/src/base/rss/rss_session.cpp @@ -45,6 +45,7 @@ #include "../profile.h" #include "../settingsstorage.h" #include "../utils/fs.h" +#include "../utils/io.h" #include "rss_article.h" #include "rss_feed.h" #include "rss_folder.h" @@ -261,33 +262,35 @@ Item *Session::itemByPath(const QString &path) const void Session::load() { - QFile itemsFile {(m_confFileStorage->storageDir() / Path(FEEDS_FILE_NAME)).data()}; - if (!itemsFile.exists()) - { - loadLegacy(); - return; - } + const int fileMaxSize = 10 * 1024 * 1024; + const Path path = m_confFileStorage->storageDir() / Path(FEEDS_FILE_NAME); - if (!itemsFile.open(QFile::ReadOnly)) + const auto readResult = Utils::IO::readFile(path, fileMaxSize); + if (!readResult) { - LogMsg(tr("Couldn't read RSS session data. File: \"%1\". Error: \"%2\"") - .arg(itemsFile.fileName(), itemsFile.errorString()), Log::WARNING); + if (readResult.error().status == Utils::IO::ReadError::NotExist) + { + loadLegacy(); + return; + } + + LogMsg(tr("Failed to read RSS session data. %1").arg(readResult.error().message), Log::WARNING); return; } QJsonParseError jsonError; - const QJsonDocument jsonDoc = QJsonDocument::fromJson(itemsFile.readAll(), &jsonError); + const QJsonDocument jsonDoc = QJsonDocument::fromJson(readResult.value(), &jsonError); if (jsonError.error != QJsonParseError::NoError) { - LogMsg(tr("Couldn't parse RSS session data. File: \"%1\". Error: \"%2\"") - .arg(itemsFile.fileName(), jsonError.errorString()), Log::WARNING); + LogMsg(tr("Failed to parse RSS session data. File: \"%1\". Error: \"%2\"") + .arg(path.toString(), jsonError.errorString()), Log::WARNING); return; } if (!jsonDoc.isObject()) { - LogMsg(tr("Couldn't load RSS session data. File: \"%1\". Error: Invalid data format.") - .arg(itemsFile.fileName()), Log::WARNING); + LogMsg(tr("Failed to load RSS session data. File: \"%1\". Error: \"Invalid data format.\"") + .arg(path.toString()), Log::WARNING); return; } diff --git a/src/base/search/searchpluginmanager.cpp b/src/base/search/searchpluginmanager.cpp index 69669fb04..a3bbc74e6 100644 --- a/src/base/search/searchpluginmanager.cpp +++ b/src/base/search/searchpluginmanager.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -517,7 +518,7 @@ void SearchPluginManager::update() nova.start(Utils::ForeignApps::pythonInfo().executableName, params, QIODevice::ReadOnly); nova.waitForFinished(); - const auto capabilities = QString::fromUtf8(nova.readAll()); + const auto capabilities = QString::fromUtf8(nova.readAllStandardOutput()); QDomDocument xmlDoc; if (!xmlDoc.setContent(capabilities)) { @@ -629,13 +630,15 @@ Path SearchPluginManager::pluginPath(const QString &name) PluginVersion SearchPluginManager::getPluginVersion(const Path &filePath) { + const int lineMaxLength = 16; + QFile pluginFile {filePath.data()}; if (!pluginFile.open(QIODevice::ReadOnly | QIODevice::Text)) return {}; while (!pluginFile.atEnd()) { - const auto line = QString::fromUtf8(pluginFile.readLine()).remove(u' '); + const auto line = QString::fromUtf8(pluginFile.readLine(lineMaxLength)).remove(u' '); if (!line.startsWith(u"#VERSION:", Qt::CaseInsensitive)) continue; const QString versionStr = line.mid(9); diff --git a/src/base/torrentfileswatcher.cpp b/src/base/torrentfileswatcher.cpp index 37262d165..f704c35dd 100644 --- a/src/base/torrentfileswatcher.cpp +++ b/src/base/torrentfileswatcher.cpp @@ -177,33 +177,35 @@ void TorrentFilesWatcher::initWorker() void TorrentFilesWatcher::load() { - QFile confFile {(specialFolderLocation(SpecialFolder::Config) / Path(CONF_FILE_NAME)).data()}; - if (!confFile.exists()) - { - loadLegacy(); - return; - } + const int fileMaxSize = 10 * 1024 * 1024; + const Path path = specialFolderLocation(SpecialFolder::Config) / Path(CONF_FILE_NAME); - if (!confFile.open(QFile::ReadOnly)) + const auto readResult = Utils::IO::readFile(path, fileMaxSize); + if (!readResult) { - LogMsg(tr("Couldn't load Watched Folders configuration from %1. Error: %2") - .arg(confFile.fileName(), confFile.errorString()), Log::WARNING); + if (readResult.error().status == Utils::IO::ReadError::NotExist) + { + loadLegacy(); + return; + } + + LogMsg(tr("Failed to load Watched Folders configuration. %1").arg(readResult.error().message), Log::WARNING); return; } QJsonParseError jsonError; - const QJsonDocument jsonDoc = QJsonDocument::fromJson(confFile.readAll(), &jsonError); + const QJsonDocument jsonDoc = QJsonDocument::fromJson(readResult.value(), &jsonError); if (jsonError.error != QJsonParseError::NoError) { - LogMsg(tr("Couldn't parse Watched Folders configuration from %1. Error: %2") - .arg(confFile.fileName(), jsonError.errorString()), Log::WARNING); + LogMsg(tr("Failed to parse Watched Folders configuration from %1. Error: \"%2\"") + .arg(path.toString(), jsonError.errorString()), Log::WARNING); return; } if (!jsonDoc.isObject()) { - LogMsg(tr("Couldn't load Watched Folders configuration from %1. Invalid data format.") - .arg(confFile.fileName()), Log::WARNING); + LogMsg(tr("Failed to load Watched Folders configuration from %1. Error: \"Invalid data format.\"") + .arg(path.toString()), Log::WARNING); return; } @@ -426,17 +428,26 @@ void TorrentFilesWatcher::Worker::processFolder(const Path &path, const Path &wa if (filePath.hasExtension(u".magnet"_qs)) { + const int fileMaxSize = 100 * 1024 * 1024; + QFile file {filePath.data()}; if (file.open(QIODevice::ReadOnly | QIODevice::Text)) { - while (!file.atEnd()) + if (file.size() <= fileMaxSize) { - const auto line = QString::fromLatin1(file.readLine()).trimmed(); - emit magnetFound(BitTorrent::MagnetUri(line), addTorrentParams); - } + while (!file.atEnd()) + { + const auto line = QString::fromLatin1(file.readLine()).trimmed(); + emit magnetFound(BitTorrent::MagnetUri(line), addTorrentParams); + } - file.close(); - Utils::Fs::removeFile(filePath); + file.close(); + Utils::Fs::removeFile(filePath); + } + else + { + LogMsg(tr("Magnet file too big. File: %1").arg(file.errorString())); + } } else { diff --git a/src/base/utils/io.cpp b/src/base/utils/io.cpp index b5c938efc..75a3088d3 100644 --- a/src/base/utils/io.cpp +++ b/src/base/utils/io.cpp @@ -31,7 +31,9 @@ #include #include +#include #include +#include #include #include #include @@ -69,6 +71,36 @@ Utils::IO::FileDeviceOutputIterator &Utils::IO::FileDeviceOutputIterator::operat return *this; } +nonstd::expected Utils::IO::readFile(const Path &path, const qint64 maxSize, const QIODevice::OpenMode additionalMode) +{ + QFile file {path.data()}; + if (!file.open(QIODevice::ReadOnly | additionalMode)) + { + const QString message = QCoreApplication::translate("Utils::IO", "File open error. File: \"%1\". Error: \"%2\"") + .arg(file.fileName(), file.errorString()); + return nonstd::make_unexpected(ReadError {ReadError::NotExist, message}); + } + + const qint64 fileSize = file.size(); + if ((maxSize >= 0) && (fileSize > maxSize)) + { + const QString message = QCoreApplication::translate("Utils::IO", "File size exceeds limit. File: \"%1\". File size: %2. Size limit: %3") + .arg(file.fileName(), QString::number(fileSize), QString::number(maxSize)); + return nonstd::make_unexpected(ReadError {ReadError::ExceedSize, message}); + } + + // Do not use `QIODevice::readAll()` it won't stop when reading `/dev/zero` + const QByteArray data = file.read(fileSize); + if (const qint64 dataSize = data.size(); dataSize != fileSize) + { + const QString message = QCoreApplication::translate("Utils::IO", "Read size mismatch. File: \"%1\". Expected: %2. Actual: %3") + .arg(file.fileName(), QString::number(fileSize), QString::number(dataSize)); + return nonstd::make_unexpected(ReadError {ReadError::SizeMismatch, message}); + } + + return data; +} + nonstd::expected Utils::IO::saveToFile(const Path &path, const QByteArray &data) { if (const Path parentPath = path.parentPath(); !parentPath.isEmpty()) diff --git a/src/base/utils/io.h b/src/base/utils/io.h index 767381ebe..3ec29b1cc 100644 --- a/src/base/utils/io.h +++ b/src/base/utils/io.h @@ -33,6 +33,8 @@ #include +#include + #include "base/3rdparty/expected.hpp" #include "base/pathfwd.h" @@ -81,6 +83,23 @@ namespace Utils::IO int m_bufferSize = 0; }; + struct ReadError + { + enum Code + { + NotExist, + ExceedSize, + SizeMismatch + }; + + Code status = {}; + QString message; + }; + + // TODO: define a specific type for `additionalMode` + // providing `size` is explicit and is strongly recommended + nonstd::expected readFile(const Path &path, qint64 size, QIODevice::OpenMode additionalMode = {}); + nonstd::expected saveToFile(const Path &path, const QByteArray &data); nonstd::expected saveToFile(const Path &path, const lt::entry &data); } diff --git a/src/gui/aboutdialog.cpp b/src/gui/aboutdialog.cpp index 701313488..c2d199c74 100644 --- a/src/gui/aboutdialog.cpp +++ b/src/gui/aboutdialog.cpp @@ -28,11 +28,10 @@ #include "aboutdialog.h" -#include - #include "base/global.h" #include "base/path.h" #include "base/unicodestrings.h" +#include "base/utils/io.h" #include "base/utils/misc.h" #include "base/version.h" #include "ui_aboutdialog.h" @@ -75,27 +74,24 @@ AboutDialog::AboutDialog(QWidget *parent) m_ui->labelMascot->setPixmap(Utils::Gui::scaledPixmap(Path(u":/icons/mascot.png"_qs), this)); // Thanks - QFile thanksfile(u":/thanks.html"_qs); - if (thanksfile.open(QIODevice::ReadOnly | QIODevice::Text)) + if (const auto readResult = Utils::IO::readFile(Path(u":/thanks.html"_qs), -1, QIODevice::Text) + ; readResult) { - m_ui->textBrowserThanks->setHtml(QString::fromUtf8(thanksfile.readAll().constData())); - thanksfile.close(); + m_ui->textBrowserThanks->setHtml(QString::fromUtf8(readResult.value())); } // Translation - QFile translatorsfile(u":/translators.html"_qs); - if (translatorsfile.open(QIODevice::ReadOnly | QIODevice::Text)) + if (const auto readResult = Utils::IO::readFile(Path(u":/translators.html"_qs), -1, QIODevice::Text) + ; readResult) { - m_ui->textBrowserTranslation->setHtml(QString::fromUtf8(translatorsfile.readAll().constData())); - translatorsfile.close(); + m_ui->textBrowserTranslation->setHtml(QString::fromUtf8(readResult.value())); } // License - QFile licensefile(u":/gpl.html"_qs); - if (licensefile.open(QIODevice::ReadOnly | QIODevice::Text)) + if (const auto readResult = Utils::IO::readFile(Path(u":/gpl.html"_qs), -1, QIODevice::Text) + ; readResult) { - m_ui->textBrowserLicense->setHtml(QString::fromUtf8(licensefile.readAll().constData())); - licensefile.close(); + m_ui->textBrowserLicense->setHtml(QString::fromUtf8(readResult.value())); } // Software Used diff --git a/src/gui/optionsdialog.cpp b/src/gui/optionsdialog.cpp index f10c37727..74557b941 100644 --- a/src/gui/optionsdialog.cpp +++ b/src/gui/optionsdialog.cpp @@ -52,6 +52,7 @@ #include "base/rss/rss_session.h" #include "base/torrentfileguard.h" #include "base/torrentfileswatcher.h" +#include "base/utils/io.h" #include "base/utils/misc.h" #include "base/utils/net.h" #include "base/utils/password.h" @@ -1751,46 +1752,22 @@ Path OptionsDialog::getFilter() const #ifndef DISABLE_WEBUI void OptionsDialog::webUIHttpsCertChanged(const Path &path) { - const auto isCertFileValid = [&path]() -> bool - { - if (path.isEmpty()) - return false; - - QFile file {path.data()}; - if (!file.open(QIODevice::ReadOnly)) - return false; - - if (!Utils::Net::isSSLCertificatesValid(file.read(Utils::Net::MAX_SSL_FILE_SIZE))) - return false; - - return true; - }; + const auto readResult = Utils::IO::readFile(path, Utils::Net::MAX_SSL_FILE_SIZE); + const bool isCertValid = Utils::Net::isSSLCertificatesValid(readResult.value_or(QByteArray())); m_ui->textWebUIHttpsCert->setSelectedPath(path); m_ui->lblSslCertStatus->setPixmap(UIThemeManager::instance()->getScaledPixmap( - (isCertFileValid() ? u"security-high"_qs : u"security-low"_qs), 24)); + (isCertValid ? u"security-high"_qs : u"security-low"_qs), 24)); } void OptionsDialog::webUIHttpsKeyChanged(const Path &path) { - const auto isKeyFileValid = [&path]() -> bool - { - if (path.isEmpty()) - return false; - - QFile file {path.data()}; - if (!file.open(QIODevice::ReadOnly)) - return false; - - if (!Utils::Net::isSSLKeyValid(file.read(Utils::Net::MAX_SSL_FILE_SIZE))) - return false; - - return true; - }; + const auto readResult = Utils::IO::readFile(path, Utils::Net::MAX_SSL_FILE_SIZE); + const bool isKeyValid = Utils::Net::isSSLKeyValid(readResult.value_or(QByteArray())); m_ui->textWebUIHttpsKey->setSelectedPath(path); m_ui->lblSslKeyStatus->setPixmap(UIThemeManager::instance()->getScaledPixmap( - (isKeyFileValid() ? u"security-high"_qs : u"security-low"_qs), 24)); + (isKeyValid ? u"security-high"_qs : u"security-low"_qs), 24)); } bool OptionsDialog::isWebUiEnabled() const diff --git a/src/gui/previewselectdialog.cpp b/src/gui/previewselectdialog.cpp index f2f41d2d3..36f069eab 100644 --- a/src/gui/previewselectdialog.cpp +++ b/src/gui/previewselectdialog.cpp @@ -30,7 +30,6 @@ #include #include -#include #include #include #include diff --git a/src/gui/rss/automatedrssdownloader.cpp b/src/gui/rss/automatedrssdownloader.cpp index 3f8b25066..4e337ad52 100644 --- a/src/gui/rss/automatedrssdownloader.cpp +++ b/src/gui/rss/automatedrssdownloader.cpp @@ -458,15 +458,16 @@ void AutomatedRssDownloader::onImportBtnClicked() const Path path {QFileDialog::getOpenFileName( this, tr("Import RSS rules"), QDir::homePath() , u"%1;;%2"_qs.arg(m_formatFilterJSON, m_formatFilterLegacy), &selectedFilter)}; - if (!path.exists()) - return; - QFile file {path.data()}; - if (!file.open(QIODevice::ReadOnly)) + const int fileMaxSize = 10 * 1024 * 1024; + const auto readResult = Utils::IO::readFile(path, fileMaxSize); + if (!readResult) { - QMessageBox::critical( - this, tr("I/O Error") - , tr("Failed to open the file. Reason: %1").arg(file.errorString())); + if (readResult.error().status == Utils::IO::ReadError::NotExist) + return; + + QMessageBox::critical(this, tr("Import error") + , tr("Failed to read the file. %1").arg(readResult.error().message)); return; } @@ -479,13 +480,12 @@ void AutomatedRssDownloader::onImportBtnClicked() try { - RSS::AutoDownloader::instance()->importRules(file.readAll(),format); + RSS::AutoDownloader::instance()->importRules(readResult.value(), format); } catch (const RSS::ParsingError &error) { - QMessageBox::critical( - this, tr("Import Error") - , tr("Failed to import the selected rules file. Reason: %1").arg(error.message())); + QMessageBox::critical(this, tr("Import error") + , tr("Failed to import the selected rules file. Reason: %1").arg(error.message())); } } diff --git a/src/gui/uithemedialog.cpp b/src/gui/uithemedialog.cpp index a927e7839..0c4d54e80 100644 --- a/src/gui/uithemedialog.cpp +++ b/src/gui/uithemedialog.cpp @@ -30,7 +30,6 @@ #include #include -#include #include #include #include diff --git a/src/gui/uithemesource.cpp b/src/gui/uithemesource.cpp index 04b41d3d9..1a20e9083 100644 --- a/src/gui/uithemesource.cpp +++ b/src/gui/uithemesource.cpp @@ -30,30 +30,17 @@ #include "uithemesource.h" -#include #include #include #include "base/global.h" #include "base/logger.h" #include "base/profile.h" +#include "base/utils/io.h" namespace { - QByteArray readFile(const Path &filePath) - { - QFile file {filePath.data()}; - if (!file.exists()) - return {}; - - if (file.open(QIODevice::ReadOnly | QIODevice::Text)) - return file.readAll(); - - LogMsg(UIThemeSource::tr("UITheme - Failed to open \"%1\". Reason: %2") - .arg(filePath.filename(), file.errorString()) - , Log::WARNING); - return {}; - } + const qint64 FILE_MAX_SIZE = 1024 * 1024; QJsonObject parseThemeConfig(const QByteArray &data) { @@ -165,7 +152,16 @@ Path DefaultThemeSource::getIconPath(const QString &iconId, const ColorMode colo void DefaultThemeSource::loadColors() { - const QByteArray configData = readFile(m_userPath / Path(CONFIG_FILE_NAME)); + const auto readResult = Utils::IO::readFile((m_userPath / Path(CONFIG_FILE_NAME)), FILE_MAX_SIZE, QIODevice::Text); + if (!readResult) + { + if (readResult.error().status != Utils::IO::ReadError::NotExist) + LogMsg(tr("Failed to load default theme colors. %1").arg(readResult.error().message), Log::WARNING); + + return; + } + + const QByteArray configData = readResult.value(); if (configData.isEmpty()) return; @@ -233,7 +229,16 @@ Path CustomThemeSource::getIconPath(const QString &iconId, const ColorMode color QByteArray CustomThemeSource::readStyleSheet() { - return readFile(themeRootPath() / Path(STYLESHEET_FILE_NAME)); + const auto readResult = Utils::IO::readFile((themeRootPath() / Path(STYLESHEET_FILE_NAME)), FILE_MAX_SIZE, QIODevice::Text); + if (!readResult) + { + if (readResult.error().status != Utils::IO::ReadError::NotExist) + LogMsg(tr("Failed to load custom theme style sheet. %1").arg(readResult.error().message), Log::WARNING); + + return {}; + } + + return readResult.value(); } DefaultThemeSource *CustomThemeSource::defaultThemeSource() const @@ -243,7 +248,16 @@ DefaultThemeSource *CustomThemeSource::defaultThemeSource() const void CustomThemeSource::loadColors() { - const QByteArray configData = readFile(themeRootPath() / Path(CONFIG_FILE_NAME)); + const auto readResult = Utils::IO::readFile((themeRootPath() / Path(CONFIG_FILE_NAME)), FILE_MAX_SIZE, QIODevice::Text); + if (!readResult) + { + if (readResult.error().status != Utils::IO::ReadError::NotExist) + LogMsg(tr("Failed to load custom theme colors. %1").arg(readResult.error().message), Log::WARNING); + + return; + } + + const QByteArray configData = readResult.value(); if (configData.isEmpty()) return; diff --git a/src/gui/utils.cpp b/src/gui/utils.cpp index 12613ba61..0fd8e93c4 100644 --- a/src/gui/utils.cpp +++ b/src/gui/utils.cpp @@ -177,10 +177,12 @@ void Utils::Gui::openFolderSelect(const Path &path) QObject::connect(thread, &QThread::finished, thread, &QObject::deleteLater); thread->start(); #elif defined(Q_OS_UNIX) && !defined(Q_OS_MACOS) + const int lineMaxLength = 64; + QProcess proc; proc.start(u"xdg-mime"_qs, {u"query"_qs, u"default"_qs, u"inode/directory"_qs}); proc.waitForFinished(); - const auto output = QString::fromLocal8Bit(proc.readLine().simplified()); + const auto output = QString::fromLocal8Bit(proc.readLine(lineMaxLength).simplified()); if ((output == u"dolphin.desktop") || (output == u"org.kde.dolphin.desktop")) { proc.startDetached(u"dolphin"_qs, {u"--select"_qs, path.toString()}); @@ -190,7 +192,7 @@ void Utils::Gui::openFolderSelect(const Path &path) { proc.start(u"nautilus"_qs, {u"--version"_qs}); proc.waitForFinished(); - const auto nautilusVerStr = QString::fromLocal8Bit(proc.readLine()).remove(QRegularExpression(u"[^0-9.]"_qs)); + const auto nautilusVerStr = QString::fromLocal8Bit(proc.readLine(lineMaxLength)).remove(QRegularExpression(u"[^0-9.]"_qs)); using NautilusVersion = Utils::Version<3>; if (NautilusVersion::fromString(nautilusVerStr, {1, 0, 0}) > NautilusVersion(3, 28, 0)) proc.startDetached(u"nautilus"_qs, {(Fs::isDir(path) ? path.parentPath() : path).toString()}); diff --git a/src/webui/webapplication.cpp b/src/webui/webapplication.cpp index 0ef09b4ff..6fd0e4ce6 100644 --- a/src/webui/webapplication.cpp +++ b/src/webui/webapplication.cpp @@ -33,7 +33,6 @@ #include #include #include -#include #include #include #include @@ -48,6 +47,7 @@ #include "base/preferences.h" #include "base/types.h" #include "base/utils/fs.h" +#include "base/utils/io.h" #include "base/utils/misc.h" #include "base/utils/random.h" #include "base/utils/string.h" @@ -496,23 +496,32 @@ void WebApplication::sendFile(const Path &path) } } - QFile file {path.data()}; - if (!file.open(QIODevice::ReadOnly)) + const auto readResult = Utils::IO::readFile(path, MAX_ALLOWED_FILESIZE); + if (!readResult) { - qDebug("File %s was not found!", qUtf8Printable(path.toString())); - throw NotFoundHTTPError(); - } + const QString message = tr("Web server error. %1").arg(readResult.error().message); - if (file.size() > MAX_ALLOWED_FILESIZE) - { - qWarning("%s: exceeded the maximum allowed file size!", qUtf8Printable(path.toString())); - throw InternalServerErrorHTTPError(tr("Exceeded the maximum allowed file size (%1)!") - .arg(Utils::Misc::friendlyUnit(MAX_ALLOWED_FILESIZE))); - } + switch (readResult.error().status) + { + case Utils::IO::ReadError::NotExist: + qDebug("%s", qUtf8Printable(message)); + // don't write log messages here to avoid exhausting the disk space + throw NotFoundHTTPError(); - QByteArray data {file.readAll()}; - file.close(); + case Utils::IO::ReadError::ExceedSize: + qWarning("%s", qUtf8Printable(message)); + LogMsg(message, Log::WARNING); + throw InternalServerErrorHTTPError(readResult.error().message); + + case Utils::IO::ReadError::SizeMismatch: + LogMsg(message, Log::WARNING); + throw InternalServerErrorHTTPError(readResult.error().message); + } + + throw InternalServerErrorHTTPError(tr("Web server error. Unknown error.")); + } + QByteArray data = readResult.value(); const QMimeType mimeType = QMimeDatabase().mimeTypeForFileNameAndData(path.data(), data); const bool isTranslatable = !m_isAltUIUsed && mimeType.inherits(u"text/plain"_qs); diff --git a/src/webui/webui.cpp b/src/webui/webui.cpp index dc1b43596..44d321235 100644 --- a/src/webui/webui.cpp +++ b/src/webui/webui.cpp @@ -28,14 +28,13 @@ #include "webui.h" -#include - #include "base/http/server.h" #include "base/logger.h" #include "base/net/dnsupdater.h" #include "base/net/portforwarder.h" #include "base/path.h" #include "base/preferences.h" +#include "base/utils/io.h" #include "base/utils/net.h" #include "webapplication.h" @@ -85,10 +84,8 @@ void WebUI::configure() { const auto readData = [](const Path &path) -> QByteArray { - QFile file {path.data()}; - if (!file.open(QIODevice::ReadOnly)) - return {}; - return file.read(Utils::Net::MAX_SSL_FILE_SIZE); + const auto readResult = Utils::IO::readFile(path, Utils::Net::MAX_SSL_FILE_SIZE); + return readResult.value_or(QByteArray()); }; const QByteArray cert = readData(pref->getWebUIHttpsCertificatePath()); const QByteArray key = readData(pref->getWebUIHttpsKeyPath()); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e43799f25..ebdfff413 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -17,6 +17,7 @@ set(testFiles testutilscompare.cpp testutilsbytearray.cpp testutilsgzip.cpp + testutilsio.cpp testutilsstring.cpp testutilsversion.cpp ) diff --git a/test/testdata/size10.txt b/test/testdata/size10.txt new file mode 100644 index 000000000..28d14454c --- /dev/null +++ b/test/testdata/size10.txt @@ -0,0 +1 @@ +123456789 diff --git a/test/testutilsio.cpp b/test/testutilsio.cpp new file mode 100644 index 000000000..be01ba0d6 --- /dev/null +++ b/test/testutilsio.cpp @@ -0,0 +1,115 @@ +/* + * Bittorrent Client using Qt and libtorrent. + * Copyright (C) 2023 Mike Tzou (Chocobo1) + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + * In addition, as a special exception, the copyright holders give permission to + * link this program with the OpenSSL project's "OpenSSL" library (or with + * modified versions of it that use the same license as the "OpenSSL" library), + * and distribute the linked executables. You must obey the GNU General Public + * License in all respects for all of the code used other than "OpenSSL". If you + * modify file(s), you may extend this exception to your version of the file(s), + * but you are not obligated to do so. If you do not wish to do so, delete this + * exception statement from your version. + */ + +#include +#include + +#include "base/global.h" +#include "base/path.h" +#include "base/utils/io.h" + +class TestUtilsIO final : public QObject +{ + Q_OBJECT + Q_DISABLE_COPY_MOVE(TestUtilsIO) + +public: + TestUtilsIO() = default; + +private slots: + void testReadFile() const + { + const Path testFolder = Path(QString::fromUtf8(__FILE__)).parentPath() / Path(u"testdata"_qs); + + const Path size10File = testFolder / Path(u"size10.txt"_qs); + const QByteArray size10Data = QByteArrayLiteral("123456789\n"); + + { + const auto readResult = Utils::IO::readFile(size10File, 0); + QCOMPARE(readResult.has_value(), false); + QCOMPARE(readResult.error().status, Utils::IO::ReadError::ExceedSize); + QCOMPARE(readResult.error().message.isEmpty(), false); + } + { + const auto readResult = Utils::IO::readFile(size10File, 9); + QCOMPARE(readResult.has_value(), false); + QCOMPARE(readResult.error().status, Utils::IO::ReadError::ExceedSize); + QCOMPARE(readResult.error().message.isEmpty(), false); + } + { + const auto readResult = Utils::IO::readFile(size10File, 10); + QCOMPARE(readResult.has_value(), true); + QCOMPARE(readResult.value(), size10Data); + } + { + const auto readResult = Utils::IO::readFile(size10File, 11); + QCOMPARE(readResult.has_value(), true); + QCOMPARE(readResult.value(), size10Data); + } + { + const auto readResult = Utils::IO::readFile(size10File, -1); + QCOMPARE(readResult.has_value(), true); + QCOMPARE(readResult.value(), size10Data); + } + + { + const Path nonExistFile = testFolder / Path(u".non_existent_file_1234"_qs); + const auto readResult = Utils::IO::readFile(nonExistFile, 1); + QCOMPARE(readResult.has_value(), false); + QCOMPARE(readResult.error().status, Utils::IO::ReadError::NotExist); + QCOMPARE(readResult.error().message.isEmpty(), false); + } + +#ifdef Q_OS_UNIX + { + const auto readResult = Utils::IO::readFile(Path(u"/dev/null"_qs), 10); + QCOMPARE(readResult.has_value(), true); + QCOMPARE(readResult.value().length(), 0); + } + { + const auto readResult = Utils::IO::readFile(Path(u"/dev/null"_qs), -1); + QCOMPARE(readResult.has_value(), true); + QCOMPARE(readResult.value().length(), 0); + } + + { + const auto readResult = Utils::IO::readFile(Path(u"/dev/zero"_qs), 10); + QCOMPARE(readResult.has_value(), true); + QCOMPARE(readResult.value().length(), 0); + } + { + const auto readResult = Utils::IO::readFile(Path(u"/dev/zero"_qs), -1); + QCOMPARE(readResult.has_value(), true); + QCOMPARE(readResult.value().length(), 0); + } +#endif + } +}; + +QTEST_APPLESS_MAIN(TestUtilsIO) +#include "testutilsio.moc"