From e940ee596f84f8faea145fd93545950072800f77 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Fri, 15 Oct 2021 12:58:28 +0200 Subject: [PATCH 1/3] Check http result on update --- src/gui/updater/ocupdater.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/gui/updater/ocupdater.cpp b/src/gui/updater/ocupdater.cpp index d116afdbd43..ac634031024 100644 --- a/src/gui/updater/ocupdater.cpp +++ b/src/gui/updater/ocupdater.cpp @@ -308,14 +308,19 @@ void NSISUpdater::wipeUpdateData() void NSISUpdater::slotDownloadFinished() { QNetworkReply *reply = qobject_cast(sender()); + const QUrl url(reply->url()); reply->deleteLater(); + _file->close(); if (reply->error() != QNetworkReply::NoError) { setDownloadState(DownloadFailed); return; } + const auto status = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute); + if (status != 200) { + setDownloadState(DownloadFailed); + return; + } - QUrl url(reply->url()); - _file->close(); QSettings settings(ConfigFile::configFile(), QSettings::IniFormat); From c49429d0662b6c0065ece99bf238c18060818186 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Fri, 15 Oct 2021 12:59:06 +0200 Subject: [PATCH 2/3] Follow http redirects --- src/gui/updater/ocupdater.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/gui/updater/ocupdater.cpp b/src/gui/updater/ocupdater.cpp index ac634031024..8c5e91f2d4b 100644 --- a/src/gui/updater/ocupdater.cpp +++ b/src/gui/updater/ocupdater.cpp @@ -368,7 +368,9 @@ void NSISUpdater::versionInfoArrived(const UpdateInfo &info) if (QFile(_targetFile).exists()) { setDownloadState(DownloadComplete); } else { - QNetworkReply *reply = qnam()->get(QNetworkRequest(QUrl(url))); + auto request = QNetworkRequest(QUrl(url)); + request.setAttribute(QNetworkRequest::RedirectPolicyAttribute, QNetworkRequest::NoLessSafeRedirectPolicy); + QNetworkReply *reply = qnam()->get(request); connect(reply, &QIODevice::readyRead, this, &NSISUpdater::slotWriteFile); connect(reply, &QNetworkReply::finished, this, &NSISUpdater::slotDownloadFinished); setDownloadState(Downloading); From 57484ad034ec34c2e6aac8c534d9667acef5b827 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Fri, 15 Oct 2021 12:59:27 +0200 Subject: [PATCH 3/3] Add unit test for downloads --- src/gui/updater/ocupdater.cpp | 6 ++-- src/gui/updater/ocupdater.h | 8 +++-- test/testupdater.cpp | 60 +++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 test/testupdater.cpp diff --git a/src/gui/updater/ocupdater.cpp b/src/gui/updater/ocupdater.cpp index 8c5e91f2d4b..482bc581e2d 100644 --- a/src/gui/updater/ocupdater.cpp +++ b/src/gui/updater/ocupdater.cpp @@ -175,7 +175,7 @@ QString OCUpdater::statusString() const } } -int OCUpdater::downloadState() const +OCUpdater::DownloadState OCUpdater::downloadState() const { return _state; } @@ -360,12 +360,12 @@ void NSISUpdater::versionInfoArrived(const UpdateInfo &info) qCInfo(lcUpdater) << "Client is on latest version!"; setDownloadState(UpToDate); } else { - QString url = info.downloadUrl(); + const QString url = info.downloadUrl(); if (url.isEmpty()) { showNoUrlDialog(info); } else { _targetFile = ConfigFile::configPath() + url.mid(url.lastIndexOf('/') + 1); - if (QFile(_targetFile).exists()) { + if (QFile::exists(_targetFile)) { setDownloadState(DownloadComplete); } else { auto request = QNetworkRequest(QUrl(url)); diff --git a/src/gui/updater/ocupdater.h b/src/gui/updater/ocupdater.h index dbdb6be5177..c3d15f054cf 100644 --- a/src/gui/updater/ocupdater.h +++ b/src/gui/updater/ocupdater.h @@ -97,6 +97,8 @@ class OCUpdater : public Updater DownloadFailed, DownloadTimedOut, UpdateOnlyAvailableThroughSystem }; + Q_ENUM(DownloadState); + explicit OCUpdater(const QUrl &url); void setUpdateUrl(const QUrl &url); @@ -106,7 +108,7 @@ class OCUpdater : public Updater void checkForUpdate() override; QString statusString() const; - int downloadState() const; + DownloadState downloadState() const; void setDownloadState(DownloadState state); signals: @@ -134,7 +136,7 @@ private slots: private: QUrl _updateUrl; - int _state; + DownloadState _state; QNetworkAccessManager *_accessManager; QTimer *_timeoutWatchdog; /** Timer to guard the timeout of an individual network request */ UpdateInfo _updateInfo; @@ -162,6 +164,8 @@ private slots: void versionInfoArrived(const UpdateInfo &info) override; QScopedPointer _file; QString _targetFile; + + friend class TestUpdater; }; /** diff --git a/test/testupdater.cpp b/test/testupdater.cpp new file mode 100644 index 00000000000..3c053772af7 --- /dev/null +++ b/test/testupdater.cpp @@ -0,0 +1,60 @@ +/* + 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 + +#include "updater/updater.h" +#include "updater/ocupdater.h" + +namespace OCC { + +class TestUpdater : public QObject +{ + Q_OBJECT + +private slots: + void testVersionToInt() + { + auto lowVersion = Updater::Helper::versionToInt(1, 2, 80, 3000); + QCOMPARE(Updater::Helper::stringVersionToInt("1.2.80.3000"), lowVersion); + + auto highVersion = Updater::Helper::versionToInt(999, 2, 80, 3000); + auto currVersion = Updater::Helper::currentVersionToInt(); + QVERIFY(currVersion > 0); + QVERIFY(currVersion > lowVersion); + QVERIFY(currVersion < highVersion); + } + + void testDownload_data() + { + QTest::addColumn("url"); + QTest::addColumn("result"); + // a redirect to attic + QTest::newRow("redirect") << "https://download.owncloud.com/desktop/stable/ownCloud-2.2.4.6408-setup.exe" << OCUpdater::DownloadComplete; + QTest::newRow("broken url") << "https://&" << OCUpdater::DownloadFailed; + } + + void testDownload() + { + QFETCH(QString, url); + QFETCH(OCUpdater::DownloadState, result); + UpdateInfo info; + info.setDownloadUrl(url); + info.setVersionString("ownCloud 2.2.4 (build 6408)"); + // esnure we do the update + info.setVersion("100.2.4.6408"); + auto *updater = new NSISUpdater({}); + QSignalSpy downloadSpy(updater, &NSISUpdater::slotDownloadFinished); + updater->versionInfoArrived(info); + downloadSpy.wait(); + QCOMPARE(updater->downloadState(), result); + updater->deleteLater(); + } +}; +} + +QTEST_MAIN(OCC::TestUpdater) +#include "testupdater.moc"