Skip to content

Commit caffd49

Browse files
mgallienmgallien
authored
Merge pull request #8094 from nextcloud/bugfix/useNewerBulkUploadChecksumHeader
Bugfix/use newer bulk upload checksum header
2 parents 864ed8f + 9f14092 commit caffd49

File tree

7 files changed

+163
-13
lines changed

7 files changed

+163
-13
lines changed

‎src/libsync/account.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,11 @@ int Account::checksumRecalculateServerVersionMinSupportedMajor() const
796796
return checksumRecalculateRequestServerVersionMinSupportedMajor;
797797
}
798798

799+
bool Account::bulkUploadNeedsLegacyChecksumHeader() const
800+
{
801+
return serverVersionInt() < makeServerVersion(32, 0, 0);
802+
}
803+
799804
void Account::setServerVersion(const QString &version)
800805
{
801806
if (version == _serverVersion) {

‎src/libsync/account.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,8 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject
307307

308308
[[nodiscard]] int checksumRecalculateServerVersionMinSupportedMajor() const;
309309

310+
[[nodiscard]] bool bulkUploadNeedsLegacyChecksumHeader() const;
311+
310312
/** True when the server connection is using HTTP2 */
311313
bool isHttp2Supported() { return _http2Supported; }
312314
void setHttp2Supported(bool value) { _http2Supported = value; }

‎src/libsync/bulkpropagatorjob.cpp‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,8 @@ void BulkPropagatorJob::startUploadFile(SyncFileItemPtr item, UploadFileInfo fil
153153

154154
void BulkPropagatorJob::doStartUpload(SyncFileItemPtr item,
155155
UploadFileInfo fileToUpload,
156-
QByteArray transmissionChecksumHeader)
156+
QByteArray transmissionChecksumHeader,
157+
QByteArray md5ChecksumHeader)
157158
{
158159
if (propagator()->_abortRequested) {
159160
return;
@@ -211,6 +212,9 @@ void BulkPropagatorJob::doStartUpload(SyncFileItemPtr item,
211212

212213
const auto remotePath = propagator()->fullRemotePath(fileToUpload._file);
213214

215+
if (!md5ChecksumHeader.isEmpty()) {
216+
currentHeaders["X-File-MD5"] = md5ChecksumHeader;
217+
}
214218
currentHeaders[checkSumHeaderC] = transmissionChecksumHeader;
215219

216220
BulkUploadItem newUploadFile{propagator()->account(), item, fileToUpload,
@@ -306,7 +310,31 @@ void BulkPropagatorJob::slotComputeTransmissionChecksum(SyncFileItemPtr item,
306310
computeChecksum->setChecksumType(checksumType);
307311

308312
connect(computeChecksum, &ComputeChecksum::done, this, [this, item, fileToUpload] (const QByteArray &contentChecksumType, const QByteArray &contentChecksum) {
309-
slotStartUpload(item, fileToUpload, contentChecksumType, contentChecksum);
313+
if (propagator()->account()->bulkUploadNeedsLegacyChecksumHeader()) {
314+
slotComputeMd5Checksum(item, fileToUpload, contentChecksumType, contentChecksum);
315+
} else {
316+
slotStartUpload(item, fileToUpload, contentChecksumType, contentChecksum, {});
317+
}
318+
});
319+
connect(computeChecksum, &ComputeChecksum::done, computeChecksum, &QObject::deleteLater);
320+
321+
computeChecksum->start(fileToUpload._path);
322+
}
323+
324+
void BulkPropagatorJob::slotComputeMd5Checksum(SyncFileItemPtr item,
325+
UploadFileInfo fileToUpload,
326+
const QByteArray &transmissionChecksumType,
327+
const QByteArray &transmissionChecksum)
328+
{
329+
// Compute the transmission checksum.
330+
const auto computeChecksum = new ComputeChecksum(this);
331+
const auto checksumType = QByteArray{"MD5"};
332+
computeChecksum->setChecksumType(checksumType);
333+
334+
connect(computeChecksum, &ComputeChecksum::done, this, [this, item, fileToUpload, transmissionChecksumType, transmissionChecksum] (const QByteArray &contentChecksumType, const QByteArray &contentChecksum) {
335+
Q_UNUSED(contentChecksumType)
336+
337+
slotStartUpload(item, fileToUpload, transmissionChecksumType, transmissionChecksum, contentChecksum);
310338
});
311339
connect(computeChecksum, &ComputeChecksum::done, computeChecksum, &QObject::deleteLater);
312340

@@ -316,7 +344,8 @@ void BulkPropagatorJob::slotComputeTransmissionChecksum(SyncFileItemPtr item,
316344
void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item,
317345
UploadFileInfo fileToUpload,
318346
const QByteArray &transmissionChecksumType,
319-
const QByteArray &transmissionChecksum)
347+
const QByteArray &transmissionChecksum,
348+
const QByteArray &md5Checksum)
320349
{
321350
const auto transmissionChecksumHeader = makeChecksumHeader(transmissionChecksumType, transmissionChecksum);
322351

@@ -378,7 +407,7 @@ void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item,
378407
return;
379408
}
380409

381-
doStartUpload(item, fileToUpload, transmissionChecksum);
410+
doStartUpload(item, fileToUpload, transmissionChecksumHeader, md5Checksum);
382411
}
383412

384413
void BulkPropagatorJob::slotOnErrorStartFolderUnlock(SyncFileItemPtr item,

‎src/libsync/bulkpropagatorjob.h‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,17 @@ private slots:
7373
void slotComputeTransmissionChecksum(OCC::SyncFileItemPtr item,
7474
OCC::BulkPropagatorJob::UploadFileInfo fileToUpload);
7575

76+
void slotComputeMd5Checksum(SyncFileItemPtr item,
77+
UploadFileInfo fileToUpload,
78+
const QByteArray &transmissionChecksumType,
79+
const QByteArray &transmissionChecksum);
80+
7681
// transmission checksum computed, prepare the upload
7782
void slotStartUpload(OCC::SyncFileItemPtr item,
7883
OCC::BulkPropagatorJob::UploadFileInfo fileToUpload,
7984
const QByteArray &transmissionChecksumType,
80-
const QByteArray &transmissionChecksum);
85+
const QByteArray &transmissionChecksum,
86+
const QByteArray &md5Checksum);
8187

8288
// invoked on internal error to unlock a folder and failed
8389
void slotOnErrorStartFolderUnlock(OCC::SyncFileItemPtr item,
@@ -94,7 +100,8 @@ private slots:
94100
private:
95101
void doStartUpload(SyncFileItemPtr item,
96102
UploadFileInfo fileToUpload,
97-
QByteArray transmissionChecksumHeader);
103+
QByteArray transmissionChecksumHeader,
104+
QByteArray md5ChecksumHeader);
98105

99106
void adjustLastJobTimeout(AbstractNetworkJob *job,
100107
qint64 fileSize) const;

‎test/syncenginetestutils.cpp‎

Lines changed: 71 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <QJsonArray>
1616
#include <QJsonObject>
1717
#include <QJsonValue>
18+
#include <QCryptographicHash>
1819

1920
#include <memory>
2021
#include <filesystem>
@@ -522,18 +523,19 @@ void FakePutReply::abort()
522523
emit finished();
523524
}
524525

525-
FakePutMultiFileReply::FakePutMultiFileReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op, const QNetworkRequest &request, const QString &contentType, const QByteArray &putPayload, QObject *parent)
526+
FakePutMultiFileReply::FakePutMultiFileReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op, const QNetworkRequest &request, const QString &contentType, const QByteArray &putPayload, const QString &serverVersion, QObject *parent)
526527
: FakeReply { parent }
528+
, _serverVersion(serverVersion)
527529
{
528530
setRequest(request);
529531
setUrl(request.url());
530532
setOperation(op);
531533
open(QIODevice::ReadOnly);
532-
_allFileInfo = performMultiPart(remoteRootFileInfo, request, putPayload, contentType);
534+
_allFileInfo = performMultiPart(remoteRootFileInfo, request, putPayload, contentType, _serverVersion);
533535
QMetaObject::invokeMethod(this, "respond", Qt::QueuedConnection);
534536
}
535537

536-
QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRootFileInfo, const QNetworkRequest &request, const QByteArray &putPayload, const QString &contentType)
538+
QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRootFileInfo, const QNetworkRequest &request, const QByteArray &putPayload, const QString &contentType, const QString &serverVersion)
537539
{
538540
Q_UNUSED(request)
539541
QVector<FileInfo *> result;
@@ -555,8 +557,55 @@ QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRoot
555557
}
556558
const auto fileName = allHeaders[QStringLiteral("x-file-path")];
557559
const auto modtime = allHeaders[QByteArrayLiteral("x-file-mtime")].toLongLong();
560+
const auto expectedMd5Checksum = allHeaders[QStringLiteral("x-file-md5")];
561+
const auto standardChecksum = allHeaders[QStringLiteral("oc-checksum")];
558562
Q_ASSERT(!fileName.isEmpty());
559563
Q_ASSERT(modtime > 0);
564+
565+
auto components = serverVersion.split('.');
566+
const auto serverIntVersion = OCC::Account::makeServerVersion(components.value(0).toInt(),
567+
components.value(1).toInt(),
568+
components.value(2).toInt());
569+
570+
const auto md5ChecksumMandatory = serverIntVersion < OCC::Account::makeServerVersion(32, 0, 0);
571+
572+
if (md5ChecksumMandatory) {
573+
Q_ASSERT(!expectedMd5Checksum.isEmpty());
574+
} else {
575+
Q_ASSERT(expectedMd5Checksum.isEmpty());
576+
}
577+
Q_ASSERT(!standardChecksum.isEmpty());
578+
579+
const auto standardChecksumComponents = standardChecksum.split(':');
580+
Q_ASSERT(standardChecksumComponents.size() == 2);
581+
auto standardHashAlgorithm = QCryptographicHash::Algorithm::Sha1;
582+
const auto standardHashAlgorithmString = standardChecksumComponents.at(0);
583+
if (standardHashAlgorithmString == QStringLiteral("MD5")) {
584+
standardHashAlgorithm = QCryptographicHash::Algorithm::Md5;
585+
} else if (standardHashAlgorithmString == QStringLiteral("SHA1")) {
586+
standardHashAlgorithm = QCryptographicHash::Algorithm::Sha1;
587+
} else if (standardHashAlgorithmString == QStringLiteral("SHA256")) {
588+
standardHashAlgorithm = QCryptographicHash::Algorithm::Sha256;
589+
} else if (standardHashAlgorithmString == QStringLiteral("SHA3_256")) {
590+
standardHashAlgorithm = QCryptographicHash::Algorithm::Sha3_256;
591+
} else if (standardHashAlgorithmString == QStringLiteral("Adler32")) {
592+
Q_ASSERT(false);
593+
}
594+
595+
if (md5ChecksumMandatory) {
596+
QCryptographicHash md5SumAlgorithm{QCryptographicHash::Algorithm::Md5};
597+
598+
md5SumAlgorithm.addData(onePartBody.toLatin1());
599+
const auto computedMd5Checksum = md5SumAlgorithm.result().toHex();
600+
Q_ASSERT(expectedMd5Checksum == computedMd5Checksum);
601+
}
602+
603+
QCryptographicHash standardSumAlgorithm{standardHashAlgorithm};
604+
605+
standardSumAlgorithm.addData(onePartBody.toLatin1());
606+
const auto computedStandardChecksum = standardSumAlgorithm.result().toHex();
607+
Q_ASSERT(standardChecksumComponents.at(1) == computedStandardChecksum);
608+
560609
FileInfo *fileInfo = remoteRootFileInfo.find(fileName);
561610
if (fileInfo) {
562611
fileInfo->size = onePartBody.size();
@@ -1110,7 +1159,7 @@ QNetworkReply *FakeQNAM::createRequest(QNetworkAccessManager::Operation op, cons
11101159
reply = new FakeChunkMoveReply { info, _remoteRootFileInfo, op, newRequest, this };
11111160
} else if (verb == QLatin1String("POST") || op == QNetworkAccessManager::PostOperation) {
11121161
if (contentType.startsWith(QStringLiteral("multipart/related; boundary="))) {
1113-
reply = new FakePutMultiFileReply { info, op, newRequest, contentType, outgoingData->readAll(), this };
1162+
reply = new FakePutMultiFileReply { info, op, newRequest, contentType, outgoingData->readAll(), _serverVersion, this };
11141163
}
11151164
} else if (verb == QLatin1String("LOCK") || verb == QLatin1String("UNLOCK")) {
11161165
reply = new FakeFileLockReply{info, op, newRequest, this};
@@ -1135,6 +1184,11 @@ QNetworkReply * FakeQNAM::overrideReplyWithError(QString fileName, QNetworkAcces
11351184
return reply;
11361185
}
11371186

1187+
void FakeQNAM::setServerVersion(const QString &version)
1188+
{
1189+
_serverVersion = version;
1190+
}
1191+
11381192
FakeFolder::FakeFolder(const FileInfo &fileTemplate, const OCC::Optional<FileInfo> &localFileInfo, const QString &remotePath)
11391193
: _localModifier(_tempDir.path())
11401194
{
@@ -1156,7 +1210,8 @@ FakeFolder::FakeFolder(const FileInfo &fileTemplate, const OCC::Optional<FileInf
11561210
_account->setUrl(QUrl(QStringLiteral("http://admin:admin@localhost/owncloud")));
11571211
_account->setCredentials(new FakeCredentials { _fakeQnam });
11581212
_account->setDavDisplayName(QStringLiteral("fakename"));
1159-
_account->setServerVersion(QStringLiteral("10.0.0"));
1213+
_account->setServerVersion(_serverVersion);
1214+
_fakeQnam->setServerVersion(_serverVersion);
11601215

11611216
_journalDb = std::make_unique<OCC::SyncJournalDb>(localPath() + QStringLiteral(".sync_test.db"));
11621217
_syncEngine = std::make_unique<OCC::SyncEngine>(_account, localPath(), OCC::SyncOptions{}, remotePath, _journalDb.get());
@@ -1279,6 +1334,17 @@ void FakeFolder::enableEnforceWindowsFileNameCompatibility()
12791334
});
12801335
}
12811336

1337+
void FakeFolder::setServerVersion(const QString &version)
1338+
{
1339+
if (_serverVersion == version) {
1340+
return;
1341+
}
1342+
1343+
_serverVersion = version;
1344+
_account->setServerVersion(_serverVersion);
1345+
_fakeQnam->setServerVersion(_serverVersion);
1346+
}
1347+
12821348
FileInfo FakeFolder::currentLocalState()
12831349
{
12841350
QDir rootDir { _tempDir.path() };

‎test/syncenginetestutils.h‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,9 @@ class FakePutMultiFileReply : public FakeReply
275275
{
276276
Q_OBJECT
277277
public:
278-
FakePutMultiFileReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op, const QNetworkRequest &request, const QString &contentType, const QByteArray &putPayload, QObject *parent);
278+
FakePutMultiFileReply(FileInfo &remoteRootFileInfo, QNetworkAccessManager::Operation op, const QNetworkRequest &request, const QString &contentType, const QByteArray &putPayload, const QString &serverVersion, QObject *parent);
279279

280-
static QVector<FileInfo *> performMultiPart(FileInfo &remoteRootFileInfo, const QNetworkRequest &request, const QByteArray &putPayload, const QString &contentType);
280+
static QVector<FileInfo *> performMultiPart(FileInfo &remoteRootFileInfo, const QNetworkRequest &request, const QByteArray &putPayload, const QString &contentType, const QString &serverVersion);
281281

282282
Q_INVOKABLE virtual void respond();
283283

@@ -290,6 +290,8 @@ class FakePutMultiFileReply : public FakeReply
290290
QVector<FileInfo *> _allFileInfo;
291291

292292
QByteArray _payload;
293+
294+
QString _serverVersion;
293295
};
294296

295297
class FakeMkcolReply : public FakeReply
@@ -501,6 +503,8 @@ class FakeQNAM : public QNetworkAccessManager
501503
// monitor requests and optionally provide custom replies
502504
Override _override;
503505

506+
QString _serverVersion = QStringLiteral("10.0.0");
507+
504508
public:
505509
FakeQNAM(FileInfo initialRoot);
506510
FileInfo &currentRemoteState() { return _remoteRootFileInfo; }
@@ -516,6 +520,8 @@ class FakeQNAM : public QNetworkAccessManager
516520

517521
QNetworkReply *overrideReplyWithError(QString fileName, Operation op, QNetworkRequest newRequest);
518522

523+
void setServerVersion(const QString &version);
524+
519525
protected:
520526
QNetworkReply *createRequest(Operation op, const QNetworkRequest &request,
521527
QIODevice *outgoingData = nullptr) override;
@@ -553,6 +559,7 @@ class FakeFolder
553559
OCC::AccountPtr _account;
554560
std::unique_ptr<OCC::SyncJournalDb> _journalDb;
555561
std::unique_ptr<OCC::SyncEngine> _syncEngine;
562+
QString _serverVersion = QStringLiteral("10.0.0");
556563

557564
public:
558565
FakeFolder(const FileInfo &fileTemplate, const OCC::Optional<FileInfo> &localFileInfo = {}, const QString &remotePath = {});
@@ -561,6 +568,8 @@ class FakeFolder
561568

562569
void enableEnforceWindowsFileNameCompatibility();
563570

571+
void setServerVersion(const QString &version);
572+
564573
[[nodiscard]] OCC::AccountPtr account() const { return _account; }
565574
[[nodiscard]] OCC::SyncEngine &syncEngine() const { return *_syncEngine; }
566575
[[nodiscard]] OCC::SyncJournalDb &syncJournal() const { return *_journalDb; }

‎test/testsyncengine.cpp‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,38 @@ private slots:
180180
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
181181
}
182182

183+
void testDirUploadWithDelayedAlgorithmWithNewChecksum() {
184+
FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
185+
fakeFolder.setServerVersion(QStringLiteral("32.0.0"));
186+
fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"bulkupload", "1.0"} } } });
187+
188+
ItemCompletedSpy completeSpy(fakeFolder);
189+
fakeFolder.localModifier().mkdir("Y");
190+
fakeFolder.localModifier().insert("Y/d0");
191+
fakeFolder.localModifier().mkdir("Z");
192+
fakeFolder.localModifier().insert("Z/d0");
193+
fakeFolder.localModifier().insert("A/a0");
194+
fakeFolder.localModifier().insert("B/b0");
195+
fakeFolder.localModifier().insert("r0");
196+
fakeFolder.localModifier().insert("r1");
197+
fakeFolder.syncOnce();
198+
QVERIFY(itemDidCompleteSuccessfullyWithExpectedRank(completeSpy, "Y", 0));
199+
QVERIFY(itemDidCompleteSuccessfullyWithExpectedRank(completeSpy, "Z", 1));
200+
QVERIFY(itemDidCompleteSuccessfully(completeSpy, "Y/d0"));
201+
QVERIFY(itemSuccessfullyCompletedGetRank(completeSpy, "Y/d0") > 1);
202+
QVERIFY(itemDidCompleteSuccessfully(completeSpy, "Z/d0"));
203+
QVERIFY(itemSuccessfullyCompletedGetRank(completeSpy, "Z/d0") > 1);
204+
QVERIFY(itemDidCompleteSuccessfully(completeSpy, "A/a0"));
205+
QVERIFY(itemSuccessfullyCompletedGetRank(completeSpy, "A/a0") > 1);
206+
QVERIFY(itemDidCompleteSuccessfully(completeSpy, "B/b0"));
207+
QVERIFY(itemSuccessfullyCompletedGetRank(completeSpy, "B/b0") > 1);
208+
QVERIFY(itemDidCompleteSuccessfully(completeSpy, "r0"));
209+
QVERIFY(itemSuccessfullyCompletedGetRank(completeSpy, "r0") > 1);
210+
QVERIFY(itemDidCompleteSuccessfully(completeSpy, "r1"));
211+
QVERIFY(itemSuccessfullyCompletedGetRank(completeSpy, "r1") > 1);
212+
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
213+
}
214+
183215
void testLocalDelete() {
184216
FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
185217
ItemCompletedSpy completeSpy(fakeFolder);

0 commit comments

Comments
 (0)