[stable-3.16] Bugfix/use newer bulk upload checksum header

src/libsync/account.cpp

@@ -796,6 +796,11 @@ int Account::checksumRecalculateServerVersionMinSupportedMajor() const
     return checksumRecalculateRequestServerVersionMinSupportedMajor;
 }
 
+bool Account::bulkUploadNeedsLegacyChecksumHeader() const
+{
+    return serverVersionInt() < makeServerVersion(32, 0, 0);
+}
+
 void Account::setServerVersion(const QString &version)
 {
     if (version == _serverVersion) {

src/libsync/account.h

@@ -307,6 +307,8 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject
 
     [[nodiscard]] int checksumRecalculateServerVersionMinSupportedMajor() const;
 
+    [[nodiscard]] bool bulkUploadNeedsLegacyChecksumHeader() const;
+
     /** True when the server connection is using HTTP2  */
     bool isHttp2Supported() { return _http2Supported; }
     void setHttp2Supported(bool value) { _http2Supported = value; }

src/libsync/bulkpropagatorjob.cpp

@@ -1,3 +1,3 @@
 /*
  * Copyright 2021 (c) Matthieu Gallien <matthieu.gallien@nextcloud.com>
  *
@@ -129,7 +129,8 @@
 
 void BulkPropagatorJob::doStartUpload(SyncFileItemPtr item,
                                       UploadFileInfo fileToUpload,
-                                      QByteArray transmissionChecksumHeader)
+                                      QByteArray transmissionChecksumHeader,
+                                      QByteArray md5ChecksumHeader)
 {
     if (propagator()->_abortRequested) {
         return;
@@ -187,6 +188,9 @@
 
     const auto remotePath = propagator()->fullRemotePath(fileToUpload._file);
 
+    if (!md5ChecksumHeader.isEmpty()) {
+        currentHeaders["X-File-MD5"] = md5ChecksumHeader;
+    }
     currentHeaders[checkSumHeaderC] = transmissionChecksumHeader;
 
     BulkUploadItem newUploadFile{propagator()->account(), item, fileToUpload,
@@ -279,7 +283,31 @@
     computeChecksum->setChecksumType(checksumType);
 
     connect(computeChecksum, &ComputeChecksum::done, this, [this, item, fileToUpload] (const QByteArray &contentChecksumType, const QByteArray &contentChecksum) {
-        slotStartUpload(item, fileToUpload, contentChecksumType, contentChecksum);
+        if (propagator()->account()->bulkUploadNeedsLegacyChecksumHeader()) {
+            slotComputeMd5Checksum(item, fileToUpload, contentChecksumType, contentChecksum);
+        } else {
+            slotStartUpload(item, fileToUpload, contentChecksumType, contentChecksum, {});
+        }
+    });
+    connect(computeChecksum, &ComputeChecksum::done, computeChecksum, &QObject::deleteLater);
+
+    computeChecksum->start(fileToUpload._path);
+}
+
+void BulkPropagatorJob::slotComputeMd5Checksum(SyncFileItemPtr item,
+                                               UploadFileInfo fileToUpload,
+                                               const QByteArray &transmissionChecksumType,
+                                               const QByteArray &transmissionChecksum)
+{
+    // Compute the transmission checksum.
+    const auto computeChecksum = new ComputeChecksum(this);
+    const auto checksumType = QByteArray{"MD5"};
+    computeChecksum->setChecksumType(checksumType);
+
+    connect(computeChecksum, &ComputeChecksum::done, this, [this, item, fileToUpload, transmissionChecksumType, transmissionChecksum] (const QByteArray &contentChecksumType, const QByteArray &contentChecksum) {
+        Q_UNUSED(contentChecksumType)
+
+        slotStartUpload(item, fileToUpload, transmissionChecksumType, transmissionChecksum, contentChecksum);
     });
     connect(computeChecksum, &ComputeChecksum::done, computeChecksum, &QObject::deleteLater);
 
@@ -289,7 +317,8 @@
 void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item,
                                         UploadFileInfo fileToUpload,
                                         const QByteArray &transmissionChecksumType,
-                                        const QByteArray &transmissionChecksum)
+                                        const QByteArray &transmissionChecksum,
+                                        const QByteArray &md5Checksum)
 {
     const auto transmissionChecksumHeader = makeChecksumHeader(transmissionChecksumType, transmissionChecksum);
 
@@ -351,7 +380,7 @@
         return;
     }
 
-    doStartUpload(item, fileToUpload, transmissionChecksum);
+    doStartUpload(item, fileToUpload, transmissionChecksumHeader, md5Checksum);
 }
 
 void BulkPropagatorJob::slotOnErrorStartFolderUnlock(SyncFileItemPtr item,

src/libsync/bulkpropagatorjob.h

@@ -1,3 +1,3 @@
 /*
  * Copyright 2021 (c) Matthieu Gallien <matthieu.gallien@nextcloud.com>
  *
@@ -73,11 +73,17 @@
     void slotComputeTransmissionChecksum(OCC::SyncFileItemPtr item,
                                          OCC::BulkPropagatorJob::UploadFileInfo fileToUpload);
 
+    void slotComputeMd5Checksum(SyncFileItemPtr item,
+                                UploadFileInfo fileToUpload,
+                                const QByteArray &transmissionChecksumType,
+                                const QByteArray &transmissionChecksum);
+
     // transmission checksum computed, prepare the upload
     void slotStartUpload(OCC::SyncFileItemPtr item,
                          OCC::BulkPropagatorJob::UploadFileInfo fileToUpload,
                          const QByteArray &transmissionChecksumType,
-                         const QByteArray &transmissionChecksum);
+                         const QByteArray &transmissionChecksum,
+                         const QByteArray &md5Checksum);
 
     // invoked on internal error to unlock a folder and failed
     void slotOnErrorStartFolderUnlock(OCC::SyncFileItemPtr item,
@@ -94,7 +100,8 @@
 private:
     void doStartUpload(SyncFileItemPtr item,
                        UploadFileInfo fileToUpload,
-                       QByteArray transmissionChecksumHeader);
+                       QByteArray transmissionChecksumHeader,
+                       QByteArray md5ChecksumHeader);
 
     void adjustLastJobTimeout(AbstractNetworkJob *job,
                               qint64 fileSize) const;

test/syncenginetestutils.cpp

@@ -1,3 +1,3 @@
 /*
  *    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
@@ -15,6 +15,7 @@
 #include <QJsonArray>
 #include <QJsonObject>
 #include <QJsonValue>
+#include <QCryptographicHash>
 
 #include <memory>
 #include <filesystem>
@@ -522,18 +523,19 @@
     emit finished();
 }
 
-FakePutMultiFileReply::FakePutMultiFileReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op, const QNetworkRequest &request, const QString &contentType, const QByteArray &putPayload, QObject *parent)
+FakePutMultiFileReply::FakePutMultiFileReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op, const QNetworkRequest &request, const QString &contentType, const QByteArray &putPayload, const QString &serverVersion, QObject *parent)
     : FakeReply { parent }
+    , _serverVersion(serverVersion)
 {
     setRequest(request);
     setUrl(request.url());
     setOperation(op);
     open(QIODevice::ReadOnly);
-    _allFileInfo = performMultiPart(remoteRootFileInfo, request, putPayload, contentType);
+    _allFileInfo = performMultiPart(remoteRootFileInfo, request, putPayload, contentType, _serverVersion);
     QMetaObject::invokeMethod(this, "respond", Qt::QueuedConnection);
 }
 
-QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRootFileInfo, const QNetworkRequest &request, const QByteArray &putPayload, const QString &contentType)
+QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRootFileInfo, const QNetworkRequest &request, const QByteArray &putPayload, const QString &contentType, const QString &serverVersion)
 {
     Q_UNUSED(request)
     QVector<FileInfo *> result;
@@ -555,8 +557,55 @@
         }
         const auto fileName = allHeaders[QStringLiteral("x-file-path")];
         const auto modtime = allHeaders[QByteArrayLiteral("x-file-mtime")].toLongLong();
+        const auto expectedMd5Checksum = allHeaders[QStringLiteral("x-file-md5")];
+        const auto standardChecksum = allHeaders[QStringLiteral("oc-checksum")];
         Q_ASSERT(!fileName.isEmpty());
         Q_ASSERT(modtime > 0);
+
+        auto components = serverVersion.split('.');
+        const auto serverIntVersion = OCC::Account::makeServerVersion(components.value(0).toInt(),
+                                                                      components.value(1).toInt(),
+                                                                      components.value(2).toInt());
+
+        const auto md5ChecksumMandatory = serverIntVersion < OCC::Account::makeServerVersion(32, 0, 0);
+
+        if (md5ChecksumMandatory) {
+            Q_ASSERT(!expectedMd5Checksum.isEmpty());
+        } else {
+            Q_ASSERT(expectedMd5Checksum.isEmpty());
+        }
+        Q_ASSERT(!standardChecksum.isEmpty());
+
+        const auto standardChecksumComponents = standardChecksum.split(':');
+        Q_ASSERT(standardChecksumComponents.size() == 2);
+        auto standardHashAlgorithm = QCryptographicHash::Algorithm::Sha1;
+        const auto standardHashAlgorithmString = standardChecksumComponents.at(0);
+        if (standardHashAlgorithmString == QStringLiteral("MD5")) {
+            standardHashAlgorithm = QCryptographicHash::Algorithm::Md5;
+        } else if (standardHashAlgorithmString == QStringLiteral("SHA1")) {
+            standardHashAlgorithm = QCryptographicHash::Algorithm::Sha1;
+        } else if (standardHashAlgorithmString == QStringLiteral("SHA256")) {
+            standardHashAlgorithm = QCryptographicHash::Algorithm::Sha256;
+        } else if (standardHashAlgorithmString == QStringLiteral("SHA3_256")) {
+            standardHashAlgorithm = QCryptographicHash::Algorithm::Sha3_256;
+        } else if (standardHashAlgorithmString == QStringLiteral("Adler32")) {
+            Q_ASSERT(false);
+        }
+
+        if (md5ChecksumMandatory) {
+            QCryptographicHash md5SumAlgorithm{QCryptographicHash::Algorithm::Md5};
+
+            md5SumAlgorithm.addData(onePartBody.toLatin1());
+            const auto computedMd5Checksum = md5SumAlgorithm.result().toHex();
+            Q_ASSERT(expectedMd5Checksum == computedMd5Checksum);
+        }
+
+        QCryptographicHash standardSumAlgorithm{standardHashAlgorithm};
+
+        standardSumAlgorithm.addData(onePartBody.toLatin1());
+        const auto computedStandardChecksum = standardSumAlgorithm.result().toHex();
+        Q_ASSERT(standardChecksumComponents.at(1) == computedStandardChecksum);
+
         FileInfo *fileInfo = remoteRootFileInfo.find(fileName);
         if (fileInfo) {
             fileInfo->size = onePartBody.size();
@@ -1110,7 +1159,7 @@
             reply = new FakeChunkMoveReply { info, _remoteRootFileInfo, op, newRequest, this };
         } else if (verb == QLatin1String("POST") || op == QNetworkAccessManager::PostOperation) {
             if (contentType.startsWith(QStringLiteral("multipart/related; boundary="))) {
-                reply = new FakePutMultiFileReply { info, op, newRequest, contentType, outgoingData->readAll(), this };
+                reply = new FakePutMultiFileReply { info, op, newRequest, contentType, outgoingData->readAll(), _serverVersion, this };
             }
         } else if (verb == QLatin1String("LOCK") || verb == QLatin1String("UNLOCK")) {
             reply = new FakeFileLockReply{info, op, newRequest, this};
@@ -1135,6 +1184,11 @@
     return reply;
 }
 
+void FakeQNAM::setServerVersion(const QString &version)
+{
+    _serverVersion = version;
+}
+
 FakeFolder::FakeFolder(const FileInfo &fileTemplate, const OCC::Optional<FileInfo> &localFileInfo, const QString &remotePath)
     : _localModifier(_tempDir.path())
 {
@@ -1156,7 +1210,8 @@
     _account->setUrl(QUrl(QStringLiteral("http://admin:admin@localhost/owncloud")));
     _account->setCredentials(new FakeCredentials { _fakeQnam });
     _account->setDavDisplayName(QStringLiteral("fakename"));
-    _account->setServerVersion(QStringLiteral("10.0.0"));
+    _account->setServerVersion(_serverVersion);
+    _fakeQnam->setServerVersion(_serverVersion);
 
     _journalDb = std::make_unique<OCC::SyncJournalDb>(localPath() + QStringLiteral(".sync_test.db"));
     _syncEngine = std::make_unique<OCC::SyncEngine>(_account, localPath(), OCC::SyncOptions{}, remotePath, _journalDb.get());
@@ -1279,6 +1334,17 @@
     });
 }
 
+void FakeFolder::setServerVersion(const QString &version)
+{
+    if (_serverVersion == version) {
+        return;
+    }
+
+    _serverVersion = version;
+    _account->setServerVersion(_serverVersion);
+    _fakeQnam->setServerVersion(_serverVersion);
+}
+
 FileInfo FakeFolder::currentLocalState()
 {
     QDir rootDir { _tempDir.path() };

test/syncenginetestutils.h

@@ -1,3 +1,3 @@
 /*
  *    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
@@ -6,7 +6,7 @@
  */
 #pragma once
 
 #include "account.h"
 #include "common/result.h"
 #include "creds/abstractcredentials.h"
 #include "logger.h"
@@ -275,9 +275,9 @@
 {
     Q_OBJECT
 public:
-    FakePutMultiFileReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op, const QNetworkRequest &request, const QString &contentType, const QByteArray &putPayload, QObject *parent);
+    FakePutMultiFileReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op, const QNetworkRequest &request, const QString &contentType, const QByteArray &putPayload, const QString &serverVersion, QObject *parent);
 
-    static QVector<FileInfo *> performMultiPart(FileInfo &remoteRootFileInfo, const QNetworkRequest &request, const QByteArray &putPayload, const QString &contentType);
+    static QVector<FileInfo *> performMultiPart(FileInfo &remoteRootFileInfo, const QNetworkRequest &request, const QByteArray &putPayload, const QString &contentType, const QString &serverVersion);
 
     Q_INVOKABLE virtual void respond();
 
@@ -290,6 +290,8 @@
     QVector<FileInfo *> _allFileInfo;
 
     QByteArray _payload;
+
+    QString _serverVersion;
 };
 
 class FakeMkcolReply : public FakeReply
@@ -501,6 +503,8 @@
     // monitor requests and optionally provide custom replies
     Override _override;
 
+    QString _serverVersion = QStringLiteral("10.0.0");
+
 public:
     FakeQNAM(FileInfo initialRoot);
     FileInfo &currentRemoteState() { return _remoteRootFileInfo; }
@@ -516,6 +520,8 @@
 
     QNetworkReply *overrideReplyWithError(QString fileName, Operation op, QNetworkRequest newRequest);
 
+    void setServerVersion(const QString &version);
+
 protected:
     QNetworkReply *createRequest(Operation op, const QNetworkRequest &request,
         QIODevice *outgoingData = nullptr) override;
@@ -553,6 +559,7 @@
     OCC::AccountPtr _account;
     std::unique_ptr<OCC::SyncJournalDb> _journalDb;
     std::unique_ptr<OCC::SyncEngine> _syncEngine;
+    QString _serverVersion = QStringLiteral("10.0.0");
 
 public:
     FakeFolder(const FileInfo &fileTemplate, const OCC::Optional<FileInfo> &localFileInfo = {}, const QString &remotePath = {});
@@ -561,6 +568,8 @@
 
     void enableEnforceWindowsFileNameCompatibility();
 
+    void setServerVersion(const QString &version);
+
     [[nodiscard]] OCC::AccountPtr account() const { return _account; }
     [[nodiscard]] OCC::SyncEngine &syncEngine() const { return *_syncEngine; }
     [[nodiscard]] OCC::SyncJournalDb &syncJournal() const { return *_journalDb; }

test/testsyncengine.cpp

@@ -1,3 +1,3 @@
 /*
  *    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
@@ -5,7 +5,7 @@
  *
  */
 
 #include <QtTest>
 #include <QTextCodec>
 
 #include "syncenginetestutils.h"
@@ -180,6 +180,38 @@
         QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
     }
 
+    void testDirUploadWithDelayedAlgorithmWithNewChecksum() {
+        FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
+        fakeFolder.setServerVersion(QStringLiteral("32.0.0"));
+        fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"bulkupload", "1.0"} } } });
+
+        ItemCompletedSpy completeSpy(fakeFolder);
+        fakeFolder.localModifier().mkdir("Y");
+        fakeFolder.localModifier().insert("Y/d0");
+        fakeFolder.localModifier().mkdir("Z");
+        fakeFolder.localModifier().insert("Z/d0");
+        fakeFolder.localModifier().insert("A/a0");
+        fakeFolder.localModifier().insert("B/b0");
+        fakeFolder.localModifier().insert("r0");
+        fakeFolder.localModifier().insert("r1");
+        fakeFolder.syncOnce();
+        QVERIFY(itemDidCompleteSuccessfullyWithExpectedRank(completeSpy, "Y", 0));
+        QVERIFY(itemDidCompleteSuccessfullyWithExpectedRank(completeSpy, "Z", 1));
+        QVERIFY(itemDidCompleteSuccessfully(completeSpy, "Y/d0"));
+        QVERIFY(itemSuccessfullyCompletedGetRank(completeSpy, "Y/d0") > 1);
+        QVERIFY(itemDidCompleteSuccessfully(completeSpy, "Z/d0"));
+        QVERIFY(itemSuccessfullyCompletedGetRank(completeSpy, "Z/d0") > 1);
+        QVERIFY(itemDidCompleteSuccessfully(completeSpy, "A/a0"));
+        QVERIFY(itemSuccessfullyCompletedGetRank(completeSpy, "A/a0") > 1);
+        QVERIFY(itemDidCompleteSuccessfully(completeSpy, "B/b0"));
+        QVERIFY(itemSuccessfullyCompletedGetRank(completeSpy, "B/b0") > 1);
+        QVERIFY(itemDidCompleteSuccessfully(completeSpy, "r0"));
+        QVERIFY(itemSuccessfullyCompletedGetRank(completeSpy, "r0") > 1);
+        QVERIFY(itemDidCompleteSuccessfully(completeSpy, "r1"));
+        QVERIFY(itemSuccessfullyCompletedGetRank(completeSpy, "r1") > 1);
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
+
     void testLocalDelete() {
         FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
         ItemCompletedSpy completeSpy(fakeFolder);