From 314c00a8b77e9ec5d05d49bd44d132373ff30a26 Mon Sep 17 00:00:00 2001 From: XNG Date: Wed, 25 Dec 2019 13:59:15 +0800 Subject: [PATCH 1/3] apply http2 qt resend patch from owncloud Signed-off-by: XNG --- src/libsync/propagatedownload.cpp | 21 +-- test/syncenginetestutils.h | 10 +- test/testdownload.cpp | 248 ++++++++++++++++++++++++++++++ 3 files changed, 262 insertions(+), 17 deletions(-) create mode 100644 test/testdownload.cpp diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 56b3b1162..df182ae46 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -134,10 +134,6 @@ void GETFileJob::start() _bandwidthManager->registerDownloadJob(this); } - if (reply()->error() != QNetworkReply::NoError) { - qCWarning(lcGetJob) << " Network error: " << errorString(); - } - connect(this, &AbstractNetworkJob::networkActivity, account().data(), &Account::propagatorNetworkActivity); AbstractNetworkJob::start(); @@ -168,7 +164,6 @@ void GETFileJob::slotMetaDataChanged() // If the status code isn't 2xx, don't write the reply body to the file. // For any error: handle it when the job is finished, not here. if (httpStatus / 100 != 2) { - _device->close(); return; } if (reply()->error() != QNetworkReply::NoError) { @@ -295,15 +290,13 @@ void GETFileJob::slotReadyRead() return; } - if (_device->isOpen() && _saveBodyToFile) { - qint64 w = _device->write(buffer.constData(), r); - if (w != r) { - _errorString = _device->errorString(); - _errorStatus = SyncFileItem::NormalError; - qCWarning(lcGetJob) << "Error while writing to file" << w << r << _errorString; - reply()->abort(); - return; - } + qint64 w = _device->write(buffer.constData(), r); + if (w != r) { + _errorString = _device->errorString(); + _errorStatus = SyncFileItem::NormalError; + qCWarning(lcGetJob) << "Error while writing to file" << w << r << _errorString; + reply()->abort(); + return; } } diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h index 111fc2638..e582ff70e 100644 --- a/test/syncenginetestutils.h +++ b/test/syncenginetestutils.h @@ -716,22 +716,26 @@ class FakeErrorReply : public QNetworkReply public: FakeErrorReply(QNetworkAccessManager::Operation op, const QNetworkRequest &request, QObject *parent, int httpErrorCode) - : QNetworkReply{parent}, _httpErrorCode(httpErrorCode) { + : QNetworkReply{parent},{ setRequest(request); setUrl(request.url()); setOperation(op); open(QIODevice::ReadOnly); + setAttribute(QNetworkRequest::HttpStatusCodeAttribute, httpErrorCode); + setError(InternalServerError, "Internal Server Fake Error"); QMetaObject::invokeMethod(this, "respond", Qt::QueuedConnection); } Q_INVOKABLE void respond() { - setAttribute(QNetworkRequest::HttpStatusCodeAttribute, _httpErrorCode); - setError(InternalServerError, "Internal Server Fake Error"); emit metaDataChanged(); emit readyRead(); // finishing can come strictly after readyRead was called QTimer::singleShot(5, this, &FakeErrorReply::slotSetFinished); } + + // make public to give tests easy interface + using QNetworkReply::setError; + using QNetworkReply::setAttribute; public slots: void slotSetFinished() { diff --git a/test/testdownload.cpp b/test/testdownload.cpp new file mode 100644 index 000000000..ec451e07b --- /dev/null +++ b/test/testdownload.cpp @@ -0,0 +1,248 @@ +/* + * 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 "syncenginetestutils.h" +#include +#include + +using namespace OCC; + +static constexpr qint64 stopAfter = 3'123'668; + +/* A FakeGetReply that sends max 'fakeSize' bytes, but whose ContentLength has the corect size */ +class BrokenFakeGetReply : public FakeGetReply +{ + Q_OBJECT +public: + using FakeGetReply::FakeGetReply; + int fakeSize = stopAfter; + + qint64 bytesAvailable() const override + { + if (aborted) + return 0; + return std::min(size, fakeSize) + QIODevice::bytesAvailable(); + } + + qint64 readData(char *data, qint64 maxlen) override + { + qint64 len = std::min(qint64{ fakeSize }, maxlen); + std::fill_n(data, len, payload); + size -= len; + fakeSize -= len; + return len; + } +}; + + +SyncFileItemPtr getItem(const QSignalSpy &spy, const QString &path) +{ + for (const QList &args : spy) { + auto item = args[0].value(); + if (item->destination() == path) + return item; + } + return {}; +} + + +class TestDownload : public QObject +{ + Q_OBJECT + +private slots: + + void testResume() + { + FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; + fakeFolder.syncEngine().setIgnoreHiddenFiles(true); + QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); + auto size = 30 * 1000 * 1000; + fakeFolder.remoteModifier().insert("A/a0", size); + + // First, download only the first 3 MB of the file + fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *) -> QNetworkReply * { + if (op == QNetworkAccessManager::GetOperation && request.url().path().endsWith("A/a0")) { + return new BrokenFakeGetReply(fakeFolder.remoteModifier(), op, request, this); + } + return nullptr; + }); + + QVERIFY(!fakeFolder.syncOnce()); // The sync must fail because not all the file was downloaded + QCOMPARE(getItem(completeSpy, "A/a0")->_status, SyncFileItem::SoftError); + QCOMPARE(getItem(completeSpy, "A/a0")->_errorString, QString("The file could not be downloaded completely.")); + QVERIFY(fakeFolder.syncEngine().isAnotherSyncNeeded()); + + // Now, we need to restart, this time, it should resume. + QByteArray ranges; + fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *) -> QNetworkReply * { + if (op == QNetworkAccessManager::GetOperation && request.url().path().endsWith("A/a0")) { + ranges = request.rawHeader("Range"); + } + return nullptr; + }); + QVERIFY(fakeFolder.syncOnce()); // now this succeeds + QCOMPARE(ranges, QByteArray("bytes=" + QByteArray::number(stopAfter) + "-")); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + } + + void testErrorMessage () { + // This test's main goal is to test that the error string from the server is shown in the UI + + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + fakeFolder.syncEngine().setIgnoreHiddenFiles(true); + QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); + auto size = 3'500'000; + fakeFolder.remoteModifier().insert("A/broken", size); + + QByteArray serverMessage = "The file was not downloaded because the tests wants so!"; + + // First, download only the first 3 MB of the file + fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *) -> QNetworkReply * { + if (op == QNetworkAccessManager::GetOperation && request.url().path().endsWith("A/broken")) { + return new FakeErrorReply(op, request, this, 400, + "\n" + "\n" + "Sabre\\DAV\\Exception\\Forbidden\n" + ""+serverMessage+"\n" + ""); + } + return nullptr; + }); + + bool timedOut = false; + QTimer::singleShot(10000, &fakeFolder.syncEngine(), [&]() { timedOut = true; fakeFolder.syncEngine().abort(); }); + QVERIFY(!fakeFolder.syncOnce()); // Fail because A/broken + QVERIFY(!timedOut); + QCOMPARE(getItem(completeSpy, "A/broken")->_status, SyncFileItem::NormalError); + QVERIFY(getItem(completeSpy, "A/broken")->_errorString.contains(serverMessage)); + } + + void serverMaintenence() { + // Server in maintenance must abort the sync. + + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + fakeFolder.remoteModifier().insert("A/broken"); + fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *) -> QNetworkReply * { + if (op == QNetworkAccessManager::GetOperation) { + return new FakeErrorReply(op, request, this, 503, + "\n" + "\n" + "Sabre\\DAV\\Exception\\ServiceUnavailable\n" + "System in maintenance mode.\n" + ""); + } + return nullptr; + }); + + QSignalSpy completeSpy(&fakeFolder.syncEngine(), &SyncEngine::itemCompleted); + QVERIFY(!fakeFolder.syncOnce()); // Fail because A/broken + // FatalError means the sync was aborted, which is what we want + QCOMPARE(getItem(completeSpy, "A/broken")->_status, SyncFileItem::FatalError); + QVERIFY(getItem(completeSpy, "A/broken")->_errorString.contains("System in maintenance mode")); + } + + void testMoveFailsInAConflict() { +#ifdef Q_OS_WIN + QSKIP("Not run on windows because permission on directory does not do what is expected"); +#endif + // Test for https://github.com/owncloud/client/issues/7015 + // We want to test the case in which the renaming of the original to the conflict file succeeds, + // but renaming the temporary file fails. + // This tests uses the fact that a "touchedFile" notification will be sent at the right moment. + // Note that there will be first a notification on the file and the conflict file before. + + FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; + fakeFolder.syncEngine().setIgnoreHiddenFiles(true); + fakeFolder.remoteModifier().setContents("A/a1", 'A'); + fakeFolder.localModifier().setContents("A/a1", 'B'); + + bool propConnected = false; + QString conflictFile; + auto transProgress = connect(&fakeFolder.syncEngine(), &SyncEngine::transmissionProgress, + [&](const ProgressInfo &pi) { + auto propagator = fakeFolder.syncEngine().getPropagator(); + if (pi.status() != ProgressInfo::Propagation || propConnected || !propagator) + return; + propConnected = true; + connect(propagator.data(), &OwncloudPropagator::touchedFile, [&](const QString &s) { + if (s.contains("conflicted copy")) { + QCOMPARE(conflictFile, QString()); + conflictFile = s; + return; + } + if (!conflictFile.isEmpty()) { + // Check that the temporary file is still there + QCOMPARE(QDir(fakeFolder.localPath() + "A/").entryList({"*.~*"}, QDir::Files | QDir::Hidden).count(), 1); + // Set the permission to read only on the folder, so the rename of the temporary file will fail + QFile(fakeFolder.localPath() + "A/").setPermissions(QFile::Permissions(0x5555)); + } + }); + }); + + QVERIFY(!fakeFolder.syncOnce()); // The sync must fail because the rename failed + QVERIFY(!conflictFile.isEmpty()); + + // restore permissions + QFile(fakeFolder.localPath() + "A/").setPermissions(QFile::Permissions(0x7777)); + + QObject::disconnect(transProgress); + fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &, QIODevice *) -> QNetworkReply * { + if (op == QNetworkAccessManager::GetOperation) + QTest::qFail("There shouldn't be any download", __FILE__, __LINE__); + return nullptr; + }); + QVERIFY(fakeFolder.syncOnce()); + + // The a1 file is still tere and have the right content + QVERIFY(fakeFolder.currentRemoteState().find("A/a1")); + QCOMPARE(fakeFolder.currentRemoteState().find("A/a1")->contentChar, 'A'); + + QVERIFY(QFile::remove(conflictFile)); // So the comparison succeeds; + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + } + + void testHttp2Resend() { + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + fakeFolder.remoteModifier().insert("A/resendme", 300); + + QByteArray serverMessage = "Needs to be resend on a new connection!"; + int resendActual = 0; + int resendExpected = 2; + + // First, download only the first 3 MB of the file + fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *) -> QNetworkReply * { + if (op == QNetworkAccessManager::GetOperation && request.url().path().endsWith("A/resendme") && resendActual < resendExpected) { + auto errorReply = new FakeErrorReply(op, request, this, 400, "ignore this body"); + errorReply->setError(QNetworkReply::ContentReSendError, serverMessage); + errorReply->setAttribute(QNetworkRequest::HTTP2WasUsedAttribute, true); + errorReply->setAttribute(QNetworkRequest::HttpStatusCodeAttribute, QVariant()); + resendActual += 1; + return errorReply; + } + return nullptr; + }); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + QCOMPARE(resendActual, 2); + + fakeFolder.remoteModifier().appendByte("A/resendme"); + resendActual = 0; + resendExpected = 10; + + QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &))); + QVERIFY(!fakeFolder.syncOnce()); + QCOMPARE(resendActual, 4); // the 4th fails because it only resends 3 times + QCOMPARE(getItem(completeSpy, "A/resendme")->_status, SyncFileItem::NormalError); + QVERIFY(getItem(completeSpy, "A/resendme")->_errorString.contains(serverMessage)); + } +}; + +QTEST_GUILESS_MAIN(TestDownload) +#include "testdownload.moc" From d87a88e39fb9052779bcb7a0f7d4dc3e514e9902 Mon Sep 17 00:00:00 2001 From: XNG Date: Wed, 25 Dec 2019 14:13:03 +0800 Subject: [PATCH 2/3] apply http2 qt resend patch from owncloud Signed-off-by: XNG --- test/syncenginetestutils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h index e582ff70e..439f428f3 100644 --- a/test/syncenginetestutils.h +++ b/test/syncenginetestutils.h @@ -716,7 +716,7 @@ class FakeErrorReply : public QNetworkReply public: FakeErrorReply(QNetworkAccessManager::Operation op, const QNetworkRequest &request, QObject *parent, int httpErrorCode) - : QNetworkReply{parent},{ + QNetworkReply{parent},{ setRequest(request); setUrl(request.url()); setOperation(op); From 768cf7e1aeb135a72d223b973c2cb964e8701a68 Mon Sep 17 00:00:00 2001 From: XNG Date: Wed, 25 Dec 2019 14:36:25 +0800 Subject: [PATCH 3/3] apply http2 qt resend patch from owncloud Signed-off-by: XNG --- test/syncenginetestutils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h index 439f428f3..4a436569a 100644 --- a/test/syncenginetestutils.h +++ b/test/syncenginetestutils.h @@ -716,7 +716,7 @@ class FakeErrorReply : public QNetworkReply public: FakeErrorReply(QNetworkAccessManager::Operation op, const QNetworkRequest &request, QObject *parent, int httpErrorCode) - QNetworkReply{parent},{ + : QNetworkReply{parent}, _httpErrorCode(httpErrorCode){ setRequest(request); setUrl(request.url()); setOperation(op);