From 493b5260e4148f47cb7e7e9da3f285d511d86f7a Mon Sep 17 00:00:00 2001 From: Juergen Kellerer Date: Fri, 22 Sep 2023 13:40:49 +0200 Subject: [PATCH] Upload: Auto limit max chunk size on HTTP-413 * Using Account to remember max and last dynamic chunk size * Start sensing max upload size at 100mb (on first failure) * Handle connection close error like HTTP-413 if upload-size > 100mb * Updated tests that relied on boundless SyncOptions Signed-off-by: Juergen Kellerer --- src/cmd/cmd.cpp | 2 +- src/gui/folder.cpp | 6 +- src/gui/socketapi/socketuploadjob.cpp | 2 +- src/libsync/account.h | 21 +++++ src/libsync/configfile.cpp | 9 ++- src/libsync/owncloudpropagator.cpp | 7 +- src/libsync/propagateupload.cpp | 43 +++++++++++ src/libsync/propagateupload.h | 2 +- src/libsync/propagateuploadng.cpp | 24 +++--- src/libsync/propagateuploadv1.cpp | 1 + src/libsync/syncfileitem.h | 1 + src/libsync/syncoptions.cpp | 66 ++++++++++++---- src/libsync/syncoptions.h | 61 ++++++++++----- test/testaccount.cpp | 41 ++++++++++ test/testchunkingng.cpp | 83 +++++++++++--------- test/testnextcloudpropagator.cpp | 42 ++++++++++ test/testsyncengine.cpp | 107 ++++++++++++++++++++++++-- translations/client_en.ts | 5 ++ 18 files changed, 427 insertions(+), 96 deletions(-) diff --git a/src/cmd/cmd.cpp b/src/cmd/cmd.cpp index 2764f7255d19f..838b33c28d497 100644 --- a/src/cmd/cmd.cpp +++ b/src/cmd/cmd.cpp @@ -531,7 +531,7 @@ int main(int argc, char **argv) SyncOptions opt; opt.fillFromEnvironmentVariables(); - opt.verifyChunkSizes(); + opt.fillFromAccount(account); SyncEngine engine(account, options.source_dir, opt, folder, &db); engine.setIgnoreHiddenFiles(options.ignoreHiddenFiles); engine.setNetworkLimits(options.uplimit, options.downlimit); diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index e4984f53a7c70..2cdcd4ef05497 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -28,6 +28,7 @@ #include "syncresult.h" #include "clientproxy.h" #include "syncengine.h" +#include "owncloudpropagator.h" #include "syncrunfilelog.h" #include "socketapi/socketapi.h" #include "theme.h" @@ -1096,16 +1097,15 @@ SyncOptions Folder::initializeSyncOptions() const opt._confirmExternalStorage = cfgFile.confirmExternalStorage(); opt._moveFilesToTrash = cfgFile.moveToTrash(); opt._vfs = _vfs; - opt._parallelNetworkJobs = _accountState->account()->isHttp2Supported() ? 20 : 6; // Chunk V2: Size of chunks must be between 5MB and 5GB, except for the last chunk which can be smaller opt.setMinChunkSize(cfgFile.minChunkSize()); opt.setMaxChunkSize(cfgFile.maxChunkSize()); - opt._initialChunkSize = ::qBound(opt.minChunkSize(), cfgFile.chunkSize(), opt.maxChunkSize()); + opt.setInitialChunkSize(cfgFile.chunkSize()); opt._targetChunkUploadDuration = cfgFile.targetChunkUploadDuration(); opt.fillFromEnvironmentVariables(); - opt.verifyChunkSizes(); + opt.fillFromAccount(_accountState->account()); return opt; } diff --git a/src/gui/socketapi/socketuploadjob.cpp b/src/gui/socketapi/socketuploadjob.cpp index 6e4ebc9689099..9c2f94a819d3f 100644 --- a/src/gui/socketapi/socketuploadjob.cpp +++ b/src/gui/socketapi/socketuploadjob.cpp @@ -55,7 +55,7 @@ SocketUploadJob::SocketUploadJob(const QSharedPointer &job) SyncOptions opt; opt.fillFromEnvironmentVariables(); - opt.verifyChunkSizes(); + opt.fillFromAccount(account->account()); _engine = new SyncEngine(account->account(), Utility::trailingSlashPath(_localPath), opt, _remotePath, _db); _engine->setParent(_db); diff --git a/src/libsync/account.h b/src/libsync/account.h index 523fc7137cd9d..a84259d7d4f12 100644 --- a/src/libsync/account.h +++ b/src/libsync/account.h @@ -16,6 +16,7 @@ #ifndef SERVERCONNECTION_H #define SERVERCONNECTION_H +#include #include #include #include @@ -276,6 +277,24 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject bool isHttp2Supported() { return _http2Supported; } void setHttp2Supported(bool value) { _http2Supported = value; } + /** Max allowed request size in bytes that the server accepts. Is <= 0 if not limited */ + qint64 getMaxRequestSize() const { return _maxRequestSize; } + void setMaxRequestSize(qint64 value) { _maxRequestSize = value; } + void setMaxRequestSizeIfLower(qint64 value) { + while (true) { + const auto size = _maxRequestSize; + + if (size > 0 && value >= size) + break; + if (_maxRequestSize.testAndSetOrdered(size, value)) + break; + } + } + + /** Last successful chunk size in bytes, used to speedup auto-sensing. */ + qint64 getLastChunkSize() const { return _lastChunkSize; } + void setLastChunkSize(qint64 value) { _lastChunkSize = value; } + void clearCookieJar(); void lendCookieJarTo(QNetworkAccessManager *guest); QString cookieJarPath(); @@ -421,6 +440,8 @@ private slots: QSharedPointer _am; QScopedPointer _credentials; bool _http2Supported = false; + QAtomicInteger _maxRequestSize = -1; + QAtomicInteger _lastChunkSize = -1; /// Certificates that were explicitly rejected by the user QList _rejectedCertificates; diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index 60cb2cc71aeba..6383faa715359 100644 --- a/src/libsync/configfile.cpp +++ b/src/libsync/configfile.cpp @@ -15,6 +15,7 @@ #include "config.h" #include "configfile.h" +#include "syncoptions.h" #include "theme.h" #include "version.h" #include "common/utility.h" @@ -246,25 +247,25 @@ int ConfigFile::timeout() const qint64 ConfigFile::chunkSize() const { QSettings settings(configFile(), QSettings::IniFormat); - return settings.value(QLatin1String(chunkSizeC), 10LL * 1000LL * 1000LL).toLongLong(); // default to 10 MB + return settings.value(QLatin1String(chunkSizeC), SyncOptions::defaultChunkSize).toLongLong(); // default to 10 MB } qint64 ConfigFile::maxChunkSize() const { QSettings settings(configFile(), QSettings::IniFormat); - return settings.value(QLatin1String(maxChunkSizeC), 5LL * 1000LL * 1000LL * 1000LL).toLongLong(); // default to 5000 MB + return settings.value(QLatin1String(maxChunkSizeC), SyncOptions::chunkV2MaxChunkSize).toLongLong(); // default to 5 GB } qint64 ConfigFile::minChunkSize() const { QSettings settings(configFile(), QSettings::IniFormat); - return settings.value(QLatin1String(minChunkSizeC), 5LL * 1000LL * 1000LL).toLongLong(); // default to 5 MB + return settings.value(QLatin1String(minChunkSizeC), SyncOptions::chunkV2MinChunkSize).toLongLong(); // default to 5 MB } chrono::milliseconds ConfigFile::targetChunkUploadDuration() const { QSettings settings(configFile(), QSettings::IniFormat); - return millisecondsValue(settings, targetChunkUploadDurationC, chrono::minutes(1)); + return millisecondsValue(settings, targetChunkUploadDurationC, SyncOptions::chunkV2TargetChunkUploadDuration); } void ConfigFile::setOptionalServerNotifications(bool show) diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 488a83a3f0ff5..04081600d0067 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -395,8 +395,8 @@ std::unique_ptr OwncloudPropagator::createUploadJob(S { auto job = std::unique_ptr{}; - if (item->_size > syncOptions()._initialChunkSize && account()->capabilities().chunkingNg()) { - // Item is above _initialChunkSize, thus will be classified as to be chunked + if (item->_size > syncOptions().initialChunkSize() && account()->capabilities().chunkingNg()) { + // Item is above initialChunkSize(), thus will be classified as to be chunked job = std::make_unique(this, item); } else { job = std::make_unique(this, item); @@ -706,7 +706,8 @@ const SyncOptions &OwncloudPropagator::syncOptions() const void OwncloudPropagator::setSyncOptions(const SyncOptions &syncOptions) { _syncOptions = syncOptions; - _chunkSize = syncOptions._initialChunkSize; + _syncOptions.fillFromAccount(account()); + _chunkSize = _syncOptions.initialChunkSize(); } bool OwncloudPropagator::localFileNameClash(const QString &relFile) diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index ca08042eeaf20..eeabd5bb64388 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -676,6 +676,49 @@ void PropagateUploadFileCommon::commonErrorHandling(AbstractNetworkJob *job) SyncFileItem::Status status = classifyError(job->reply()->error(), _item->_httpErrorCode, &propagator()->_anotherSyncNeeded, replyContent); + // Request entity to large: Chunk size is larger than supported by the server + if (status == SyncFileItem::NormalError && _item->_requestBodySize > 0) { + const auto opts = propagator()->syncOptions(); + const auto requestSize = _item->_requestBodySize; + + // Check if remote host closed the connection after an upload larger than SyncOptions::maxChunkSizeAfterFailure + const bool isConnectionClosedOnLargeUpload = job->reply()->error() == QNetworkReply::RemoteHostClosedError + && requestSize > SyncOptions::maxChunkSizeAfterFailure; + + if (isConnectionClosedOnLargeUpload) { + qCInfo(lcPropagateUpload()) << "Remote host closed the connection when uploading" + << Utility::octetsToString(requestSize) << "." + << "Treating the error like HTTP-413 (request entity too large)"; + } + + // Calculate new upload limit and set it in propagator()->account() + // Sent bytes is no indicator how much is allowed by the server as requests must be consumed fully. + if (_item->_httpErrorCode == 413 || isConnectionClosedOnLargeUpload) { + const auto maxRequestSize = opts.toValidChunkSize( + requestSize > SyncOptions::maxChunkSizeAfterFailure + ? SyncOptions::maxChunkSizeAfterFailure + : requestSize / 2 + ); + + // Set new upload limit + propagator()->account()->setMaxRequestSizeIfLower(maxRequestSize); + + // Apply to this propagator (limit is applied in setSyncOptions()) + propagator()->setSyncOptions(opts); + const auto adjustedOpts = propagator()->syncOptions(); + + // Trigger retry + if (adjustedOpts.maxChunkSize() < requestSize) { + propagator()->_anotherSyncNeeded = true; + status = SyncFileItem::SoftError; + } + + errorString = tr("Upload of %1 exceeds the max upload size allowed by the server. Reducing max upload size to %2") + .arg(Utility::octetsToString(requestSize)) + .arg(Utility::octetsToString(adjustedOpts.maxChunkSize())); + } + } + // Insufficient remote storage. if (_item->_httpErrorCode == 507) { // Update the quota expectation diff --git a/src/libsync/propagateupload.h b/src/libsync/propagateupload.h index e12e4500a5efc..8d4177f48f7ee 100644 --- a/src/libsync/propagateupload.h +++ b/src/libsync/propagateupload.h @@ -351,7 +351,7 @@ class PropagateUploadFileV1 : public PropagateUploadFileCommon [[nodiscard]] qint64 chunkSize() const { // Old chunking does not use dynamic chunking algorithm, and does not adjusts the chunk size respectively, // thus this value should be used as the one classifing item to be chunked - return propagator()->syncOptions()._initialChunkSize; + return propagator()->syncOptions().initialChunkSize(); } public: diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp index 4c3ce698308fe..9c4b273346dc6 100644 --- a/src/libsync/propagateuploadng.cpp +++ b/src/libsync/propagateuploadng.cpp @@ -403,6 +403,7 @@ void PropagateUploadFileNG::slotPutFinished() if (err != QNetworkReply::NoError) { _item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); _item->_requestId = job->requestId(); + _item->_requestBodySize = job->device()->size(); commonErrorHandling(job); return; } @@ -413,10 +414,9 @@ void PropagateUploadFileNG::slotPutFinished() // // Dynamic chunk sizing is enabled if the server configured a // target duration for each chunk upload. - auto targetDuration = propagator()->syncOptions()._targetChunkUploadDuration; - if (targetDuration.count() > 0) { - auto uploadTime = ++job->msSinceStart(); // add one to avoid div-by-zero - qint64 predictedGoodSize = (_currentChunkSize * targetDuration) / uploadTime; + if (const auto opts = propagator()->syncOptions(); opts.isDynamicChunkSize()) { + const auto uploadTime = job->msSinceStart(); + const auto predictedGoodSize = opts.predictedGoodChunkSize(_currentChunkSize, uploadTime); // The whole targeting is heuristic. The predictedGoodSize will fluctuate // quite a bit because of external factors (like available bandwidth) @@ -424,15 +424,21 @@ void PropagateUploadFileNG::slotPutFinished() // // We use an exponential moving average here as a cheap way of smoothing // the chunk sizes a bit. - qint64 targetSize = propagator()->_chunkSize / 2 + predictedGoodSize / 2; + const auto targetSize = propagator()->_chunkSize / 2 + predictedGoodSize / 2; // Adjust the dynamic chunk size _chunkSize used for sizing of the item's chunks to be send - propagator()->_chunkSize = ::qBound(propagator()->syncOptions().minChunkSize(), targetSize, propagator()->syncOptions().maxChunkSize()); + propagator()->_chunkSize = opts.toValidChunkSize(targetSize); qCInfo(lcPropagateUploadNG) << "Chunked upload of" << _currentChunkSize << "bytes took" << uploadTime.count() - << "ms, desired is" << targetDuration.count() << "ms, expected good chunk size is" - << predictedGoodSize << "bytes and nudged next chunk size to " - << propagator()->_chunkSize << "bytes"; + << "ms, desired is" << opts._targetChunkUploadDuration.count() << "ms, expected" + << "good chunk size is" << predictedGoodSize << "bytes and nudged next chunk size" + << "to" << propagator()->_chunkSize << "bytes"; + + // Remembering the new chunk size in account so that it can be reused + const auto account = propagator()->account(); + qCDebug(lcPropagateUploadNG) << "Reusing chunk size of" << propagator()->_chunkSize << "bytes" + << "in the next sync with" << account->displayName(); + account->setLastChunkSize(propagator()->_chunkSize); } _finished = _sent == _item->_size; diff --git a/src/libsync/propagateuploadv1.cpp b/src/libsync/propagateuploadv1.cpp index c51fdd3bee7a3..def626d677837 100644 --- a/src/libsync/propagateuploadv1.cpp +++ b/src/libsync/propagateuploadv1.cpp @@ -215,6 +215,7 @@ void PropagateUploadFileV1::slotPutFinished() _item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt(); _item->_responseTimeStamp = job->responseTimestamp(); _item->_requestId = job->requestId(); + _item->_requestBodySize = job->device()->size(); QNetworkReply::NetworkError err = job->reply()->error(); if (err != QNetworkReply::NoError) { commonErrorHandling(job); diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index eb263f3424bfc..a56f6a3e3bd39 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -287,6 +287,7 @@ class OWNCLOUDSYNC_EXPORT SyncFileItem QString _errorString; // Contains a string only in case of error QByteArray _responseTimeStamp; QByteArray _requestId; // X-Request-Id of the failed request + qint64 _requestBodySize = -1; // the number of bytes sent. -1 if unknown. quint32 _affectedItems = 1; // the number of affected items by the operation on this item. // usually this value is 1, but for removes on dirs, it might be much higher. diff --git a/src/libsync/syncoptions.cpp b/src/libsync/syncoptions.cpp index c11d3a9bba2b1..34442b5884931 100644 --- a/src/libsync/syncoptions.cpp +++ b/src/libsync/syncoptions.cpp @@ -13,6 +13,7 @@ */ #include "syncoptions.h" +#include "account.h" #include "common/utility.h" #include @@ -31,9 +32,10 @@ qint64 SyncOptions::minChunkSize() const return _minChunkSize; } -void SyncOptions::setMinChunkSize(const qint64 minChunkSize) +void SyncOptions::setMinChunkSize(const qint64 value) { - _minChunkSize = ::qBound(_minChunkSize, minChunkSize, _maxChunkSize); + _minChunkSize = qBound(chunkV2MinChunkSize, value, _maxChunkSize); + setInitialChunkSize(_initialChunkSize); } qint64 SyncOptions::maxChunkSize() const @@ -41,24 +43,35 @@ qint64 SyncOptions::maxChunkSize() const return _maxChunkSize; } -void SyncOptions::setMaxChunkSize(const qint64 maxChunkSize) +void SyncOptions::setMaxChunkSize(const qint64 value) { - _maxChunkSize = ::qBound(_minChunkSize, maxChunkSize, _maxChunkSize); + _maxChunkSize = qBound(_minChunkSize, value, chunkV2MaxChunkSize); + setInitialChunkSize(_initialChunkSize); } -void SyncOptions::fillFromEnvironmentVariables() +qint64 SyncOptions::initialChunkSize() const { - QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE"); - if (!chunkSizeEnv.isEmpty()) - _initialChunkSize = chunkSizeEnv.toUInt(); + return _initialChunkSize; +} +void SyncOptions::setInitialChunkSize(const qint64 value) +{ + _initialChunkSize = toValidChunkSize(value); +} + +void SyncOptions::fillFromEnvironmentVariables() +{ QByteArray minChunkSizeEnv = qgetenv("OWNCLOUD_MIN_CHUNK_SIZE"); if (!minChunkSizeEnv.isEmpty()) - _minChunkSize = minChunkSizeEnv.toUInt(); + setMinChunkSize(minChunkSizeEnv.toUInt()); QByteArray maxChunkSizeEnv = qgetenv("OWNCLOUD_MAX_CHUNK_SIZE"); if (!maxChunkSizeEnv.isEmpty()) - _maxChunkSize = maxChunkSizeEnv.toUInt(); + setMaxChunkSize(maxChunkSizeEnv.toUInt()); + + QByteArray chunkSizeEnv = qgetenv("OWNCLOUD_CHUNK_SIZE"); + if (!chunkSizeEnv.isEmpty()) + setInitialChunkSize(chunkSizeEnv.toUInt()); QByteArray targetChunkUploadDurationEnv = qgetenv("OWNCLOUD_TARGET_CHUNK_UPLOAD_DURATION"); if (!targetChunkUploadDurationEnv.isEmpty()) @@ -69,10 +82,37 @@ void SyncOptions::fillFromEnvironmentVariables() _parallelNetworkJobs = maxParallel; } -void SyncOptions::verifyChunkSizes() +void SyncOptions::fillFromAccount(const AccountPtr account) +{ + if (!account) { + return; + } + + if (account->isHttp2Supported() && _parallelNetworkJobs == defaultParallelNetworkJobs) { + _parallelNetworkJobs = defaultParallelNetworkJobsH2; + } + + if (const auto size = account->getMaxRequestSize(); size > 0) { + setMaxChunkSize(size); + } + + if (account->capabilities().chunkingNg()) { + // read last used chunk size and use it as initial value + if (const auto size = account->getLastChunkSize(); size > 0) { + setInitialChunkSize(size); + } + } else { + // disable dynamic chunk sizing as it is not supported for this account + _targetChunkUploadDuration = std::chrono::milliseconds(0); + } +} + +qint64 SyncOptions::predictedGoodChunkSize(const qint64 currentChunkSize, const std::chrono::milliseconds uploadTime) const { - _minChunkSize = qMin(_minChunkSize, _initialChunkSize); - _maxChunkSize = qMax(_maxChunkSize, _initialChunkSize); + if (isDynamicChunkSize() && uploadTime.count() > 0) { + return (currentChunkSize * _targetChunkUploadDuration) / uploadTime; + } + return currentChunkSize; } QRegularExpression SyncOptions::fileRegex() const diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h index 092ac6fb6f1c6..529b58dbddb4f 100644 --- a/src/libsync/syncoptions.h +++ b/src/libsync/syncoptions.h @@ -16,6 +16,7 @@ #include "owncloudlib.h" #include "common/vfs.h" +#include "common/syncjournaldb.h" #include #include @@ -35,6 +36,27 @@ class OWNCLOUDSYNC_EXPORT SyncOptions SyncOptions(); ~SyncOptions(); + /** Min chunk size and the default value */ + static constexpr auto chunkV2MinChunkSize = 5LL * 1000LL * 1000LL; // 5 MB + + /** Max chunk size and the default value */ + static constexpr auto chunkV2MaxChunkSize = 5LL * 1000LL * 1000LL * 1000LL; // 5 GB + + /** Default upload duration target for dynamically sized chunks */ + static constexpr auto chunkV2TargetChunkUploadDuration = std::chrono::minutes(1); + + /** Default initial chunk size for dynamic chunks or the chunk size for fixed size transfers */ + static constexpr auto defaultChunkSize = 10LL * 1000LL * 1000LL; // 10 MB + + /** Max possible chunk size after an upload caused HTTP-413 or RemoteHostClosedError */ + static constexpr auto maxChunkSizeAfterFailure = 100LL * 1000LL * 1000LL; // 100 MB + + /** Default value for parallel network jobs */ + static constexpr auto defaultParallelNetworkJobs = 6; + + /** Default value for parallel network jobs for HTTP/2 connections */ + static constexpr auto defaultParallelNetworkJobsH2 = 20; + /** Maximum size (in Bytes) a folder can have without asking for confirmation. * -1 means infinite */ qint64 _newBigFolderSizeLimit = -1; @@ -48,26 +70,27 @@ class OWNCLOUDSYNC_EXPORT SyncOptions /** Create a virtual file for new files instead of downloading. May not be null */ QSharedPointer _vfs; - /** The initial un-adjusted chunk size in bytes for chunked uploads, both - * for old and new chunking algorithm, which classifies the item to be chunked - * - * In chunkingNG, when dynamic chunk size adjustments are done, this is the - * starting value and is then gradually adjusted within the - * minChunkSize / maxChunkSize bounds. - */ - qint64 _initialChunkSize = 10 * 1000 * 1000; // 10MB - /** The target duration of chunk uploads for dynamic chunk sizing. * * Set to 0 it will disable dynamic chunk sizing. */ - std::chrono::milliseconds _targetChunkUploadDuration = std::chrono::minutes(1); + std::chrono::milliseconds _targetChunkUploadDuration = chunkV2TargetChunkUploadDuration; /** The maximum number of active jobs in parallel */ - int _parallelNetworkJobs = 6; + int _parallelNetworkJobs = defaultParallelNetworkJobs; - static constexpr auto chunkV2MinChunkSize = 5LL * 1000LL * 1000LL; // 5 MB - static constexpr auto chunkV2MaxChunkSize = 5LL * 1000LL * 1000LL * 1000LL; // 5 GB + /** True for dynamic chunk sizing, false if _initialChunkSize should always be used as chunk size */ + bool isDynamicChunkSize() const { return _targetChunkUploadDuration.count() > 0; }; + + /** Returns a good new chunk size based on the current size and time of the uploaded chunk */ + qint64 predictedGoodChunkSize(const qint64 currentChunkSize, const std::chrono::milliseconds uploadTime) const; + + /** Returns chunkSize after applying the configured bounds of _minChunkSize and _maxChunkSize */ + qint64 toValidChunkSize(const qint64 chunkSize) const { return qBound(_minChunkSize, chunkSize, _maxChunkSize); }; + + /** The initial chunk size in bytes for chunked uploads */ + [[nodiscard]] qint64 initialChunkSize() const; + void setInitialChunkSize(const qint64 initialChunkSize); /** The minimum chunk size in bytes for chunked uploads */ [[nodiscard]] qint64 minChunkSize() const; @@ -84,15 +107,10 @@ class OWNCLOUDSYNC_EXPORT SyncOptions */ void fillFromEnvironmentVariables(); - /** Ensure min <= initial <= max - * - * Previously min/max chunk size values didn't exist, so users might - * have setups where the chunk size exceeds the new min/max default - * values. To cope with this, adjust min/max to always include the - * initial chunk size value. + /** Reads settings from the given account when available. + * Currently reads _initialChunkSize and _maxChunkSize. */ - void verifyChunkSizes(); - + void fillFromAccount(const AccountPtr account); /** A regular expression to match file names * If no pattern is provided the default is an invalid regular expression. @@ -116,6 +134,7 @@ class OWNCLOUDSYNC_EXPORT SyncOptions */ QRegularExpression _fileRegex = QRegularExpression(QStringLiteral("(")); + qint64 _initialChunkSize = defaultChunkSize; qint64 _minChunkSize = chunkV2MinChunkSize; qint64 _maxChunkSize = chunkV2MaxChunkSize; }; diff --git a/test/testaccount.cpp b/test/testaccount.cpp index e5122b7f39ebb..090d504d86886 100644 --- a/test/testaccount.cpp +++ b/test/testaccount.cpp @@ -28,6 +28,47 @@ private slots: AccountPtr account = Account::create(); [[maybe_unused]] const auto davPath = account->davPath(); } + + void testAccountMaxRequestSize_initial_minusOne() + { + QCOMPARE( Account::create()->getMaxRequestSize(), -1 ); + } + + void testAccountMaxRequestSize_readWrite() + { + auto account = Account::create(); + QVERIFY( account->getMaxRequestSize() == -1 ); + + for (qint64 i = -2; i < 100000000000; i += 100000) { + QVERIFY( i != account->getMaxRequestSize() ); + + account->setMaxRequestSize(i); + QCOMPARE( i, account->getMaxRequestSize() ); + } + } + + void testAccountMaxRequestSize_writeIfLower() + { + using Test = std::pair; // + std::vector tests{ + Test(10000, 10000), + Test(-1, -1), // reset with -1 + Test(1, 1), + Test(2, 1), + Test(0, 0), // reset with 0 + Test(1000, 1000), + Test(1100, 1000), + Test(900, 900), + Test(80, 80), + Test(1000, 80), + }; + + auto account = Account::create(); + for (auto test : tests) { + account->setMaxRequestSizeIfLower(test.first); + QCOMPARE( test.second, account->getMaxRequestSize() ); + } + } }; QTEST_APPLESS_MAIN(TestAccount) diff --git a/test/testchunkingng.cpp b/test/testchunkingng.cpp index 913d9f12ef5d0..4145abf5cd603 100644 --- a/test/testchunkingng.cpp +++ b/test/testchunkingng.cpp @@ -44,14 +44,17 @@ static void partialUpload(FakeFolder &fakeFolder, const QString &name, qint64 si static void setChunkSize(SyncEngine &engine, qint64 size) { SyncOptions options; - options.setMaxChunkSize(size); options.setMinChunkSize(size); - options._initialChunkSize = size; + options.setMaxChunkSize(size); + options.setInitialChunkSize(size); engine.setSyncOptions(options); } class TestChunkingNG : public QObject { + static constexpr auto maxChunkSize = 5LL * 1000LL * 1000LL * 1000LL; + static constexpr auto minChunkSize = 5LL * 1000LL * 1000LL; + Q_OBJECT private slots: @@ -60,12 +63,10 @@ private slots: FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({{"dav", QVariantMap{{"chunking", "1.0"}}}}); - constexpr auto maxChunkSize = 5LL * 1000LL * 1000LL * 1000LL; - ::setChunkSize(fakeFolder.syncEngine(), 10LL * 1000LL * 1000LL * 1000LL); + ::setChunkSize(fakeFolder.syncEngine(), maxChunkSize * 2); QCOMPARE(fakeFolder.syncEngine().syncOptions().maxChunkSize(), maxChunkSize); - constexpr auto minChunkSize = 5 * 1000 * 1000; - ::setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); + ::setChunkSize(fakeFolder.syncEngine(), minChunkSize / 2); QCOMPARE(fakeFolder.syncEngine().syncOptions().minChunkSize(), minChunkSize); auto hasDestinationHeader = false; @@ -95,8 +96,8 @@ private slots: { FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); - setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); - const int size = 10 * 1000 * 1000; // 10 MB + setChunkSize(fakeFolder.syncEngine(), minChunkSize); + const int size = 5 * minChunkSize; // 25 MB fakeFolder.localModifier().insert("A/a0", size); QVERIFY(fakeFolder.syncOnce()); @@ -115,8 +116,8 @@ private slots: void testResume1() { FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); - const int size = 10 * 1000 * 1000; // 10 MB - setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); + setChunkSize(fakeFolder.syncEngine(), minChunkSize); + const int size = 5 * minChunkSize; // 25 MB partialUpload(fakeFolder, "A/a0", size); QCOMPARE(fakeFolder.uploadState().children.count(), 1); @@ -151,8 +152,8 @@ private slots: void testResume2() { FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); - setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); - const int size = 30 * 1000 * 1000; // 30 MB + setChunkSize(fakeFolder.syncEngine(), minChunkSize); + const int size = 10 * minChunkSize; // 50 MB partialUpload(fakeFolder, "A/a0", size); QCOMPARE(fakeFolder.uploadState().children.count(), 1); auto chunkingId = fakeFolder.uploadState().children.first().name; @@ -207,8 +208,8 @@ private slots: void testResume3() { FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); - const int size = 30 * 1000 * 1000; // 30 MB - setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); + setChunkSize(fakeFolder.syncEngine(), minChunkSize); + const int size = 10 * minChunkSize; // 50 MB partialUpload(fakeFolder, "A/a0", size); QCOMPARE(fakeFolder.uploadState().children.count(), 1); @@ -255,9 +256,8 @@ private slots: FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); - constexpr auto size = 30 * 1000 * 1000; // 30 MB - constexpr auto chunkSize = 5 * 1000 * 1000; // 5 MB - setChunkSize(fakeFolder.syncEngine(), chunkSize); + setChunkSize(fakeFolder.syncEngine(), minChunkSize);; // 5 MB + constexpr auto size = 10 * minChunkSize; // 50 MB partialUpload(fakeFolder, "A/a0", size); QCOMPARE(fakeFolder.uploadState().children.count(), 1); @@ -291,8 +291,8 @@ private slots: { FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ { "chunking", "1.0" } } }, { "checksums", QVariantMap{ { "supportedTypes", QStringList() << "SHA1" } } } }); - const int size = 15 * 1000 * 1000; // 15 MB - setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); + setChunkSize(fakeFolder.syncEngine(), minChunkSize);; // 5 MB + constexpr auto size = 10 * minChunkSize; // 50 MB // Make the MOVE never reply, but trigger a client-abort and apply the change remotely QObject parent; @@ -375,8 +375,8 @@ private slots: { FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ { "chunking", "1.0" } } }, { "checksums", QVariantMap{ { "supportedTypes", QStringList() << "SHA1" } } } }); - const int size = 15 * 1000 * 1000; // 15 MB - setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); + setChunkSize(fakeFolder.syncEngine(), minChunkSize);; // 5 MB + constexpr auto size = 10 * minChunkSize; // 50 MB // Make the MOVE never reply, but trigger a client-abort and apply the change remotely QObject parent; @@ -405,8 +405,8 @@ private slots: FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); - const int size = 10 * 1000 * 1000; // 10 MB - setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); + setChunkSize(fakeFolder.syncEngine(), minChunkSize);; // 5 MB + constexpr auto size = 5 * minChunkSize; // 25 MB partialUpload(fakeFolder, "A/a0", size); QCOMPARE(fakeFolder.uploadState().children.count(), 1); @@ -430,8 +430,8 @@ private slots: FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); - const int size = 10 * 1000 * 1000; // 10 MB - setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); + setChunkSize(fakeFolder.syncEngine(), minChunkSize);; // 5 MB + constexpr auto size = 5 * minChunkSize; // 25 MB partialUpload(fakeFolder, "A/a0", size); QCOMPARE(fakeFolder.uploadState().children.count(), 1); @@ -446,8 +446,8 @@ private slots: void testCreateConflictWhileSyncing() { FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); - const int size = 10 * 1000 * 1000; // 10 MB - setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); + setChunkSize(fakeFolder.syncEngine(), minChunkSize);; // 5 MB + constexpr auto size = 10 * minChunkSize; // 50 MB // Put a file on the server and download it. fakeFolder.remoteModifier().insert("A/a0", size); @@ -502,8 +502,8 @@ private slots: FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); - const int size = 10 * 1000 * 1000; // 10 MB - setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); + setChunkSize(fakeFolder.syncEngine(), minChunkSize);; // 5 MB + constexpr auto size = 5 * minChunkSize; // 25 MB fakeFolder.localModifier().insert("A/a0", size); @@ -541,8 +541,8 @@ private slots: FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); - const int size = 30 * 1000 * 1000; // 30 MB - setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); + setChunkSize(fakeFolder.syncEngine(), minChunkSize);; // 5 MB + constexpr auto size = 5 * minChunkSize; // 25 MB partialUpload(fakeFolder, "A/a0", size); QCOMPARE(fakeFolder.uploadState().children.count(), 1); auto chunkingId = fakeFolder.uploadState().children.first().name; @@ -572,8 +572,8 @@ private slots: QFETCH(bool, chunking); FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ { "chunking", "1.0" } } }, { "checksums", QVariantMap{ { "supportedTypes", QStringList() << "SHA1" } } } }); - const int size = chunking ? 1 * 1000 * 1000 : 300; - setChunkSize(fakeFolder.syncEngine(), 300 * 1000); + const int size = chunking ? 10 * minChunkSize : 300; + setChunkSize(fakeFolder.syncEngine(), minChunkSize); // Make the MOVE never reply, but trigger a client-abort and apply the change remotely QByteArray checksumHeader; @@ -627,8 +627,8 @@ private slots: QTextCodec::codecForLocale()->setCodecForLocale(QTextCodec::codecForName("UTF-8")); FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); - const int size = 5 * 1000 * 1000; - setChunkSize(fakeFolder.syncEngine(), 1 * 1000 * 1000); + setChunkSize(fakeFolder.syncEngine(), minChunkSize);; // 5 MB + constexpr auto size = 5 * minChunkSize; // 25 MB fakeFolder.localModifier().insert("A/file % \u20ac", size); QVERIFY(fakeFolder.syncOnce()); @@ -640,6 +640,19 @@ private slots: QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); } + void testAccountReceivesLastChunkSize() { + FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; + const auto account = fakeFolder.syncEngine().account(); + account->setCapabilities({ { "dav", QVariantMap{ {"chunking", "1.0"} } } }); + + const auto size = 3 * fakeFolder.syncEngine().syncOptions().initialChunkSize(); + fakeFolder.localModifier().insert("A/file", size); + + QCOMPARE(-1, account->getLastChunkSize()); + QVERIFY(fakeFolder.syncOnce()); + QVERIFY(account->getLastChunkSize() > 0); + } + // Test uploading large files (2.5GiB) void testVeryBigFiles() { FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()}; diff --git a/test/testnextcloudpropagator.cpp b/test/testnextcloudpropagator.cpp index 954c665f5f91c..608769baacf99 100644 --- a/test/testnextcloudpropagator.cpp +++ b/test/testnextcloudpropagator.cpp @@ -7,8 +7,10 @@ #include #include +#include "account.h" #include "propagatedownload.h" #include "owncloudpropagator_p.h" +#include "syncoptions.h" using namespace OCC; namespace OCC { @@ -76,6 +78,46 @@ private slots: QCOMPARE(parseEtag(test.first), QByteArray(test.second)); } } + + void testRespectsLowestPossibleChunkSize() + { + QSet __blacklist; + OwncloudPropagator propagator(Account::create(), "", "", nullptr, __blacklist); + auto opts = propagator.syncOptions(); + + opts.setMinChunkSize(0); + opts.setMaxChunkSize(0); + opts.setInitialChunkSize(0); + + propagator.setSyncOptions(opts); + + opts = propagator.syncOptions(); + QCOMPARE( opts.minChunkSize(), SyncOptions::chunkV2MinChunkSize ); + QCOMPARE( opts.initialChunkSize(), SyncOptions::chunkV2MinChunkSize ); + QCOMPARE( opts.maxChunkSize(), SyncOptions::chunkV2MinChunkSize ); + } + + void testLimitsMaxChunkSizeByAccount() + { + QSet __blacklist; + OwncloudPropagator propagator(Account::create(), "", "", nullptr, __blacklist); + auto opts = propagator.syncOptions(); + + SyncOptions defaultOpts; + QCOMPARE( opts.minChunkSize(), defaultOpts.minChunkSize() ); + QVERIFY( opts.minChunkSize() < defaultOpts.maxChunkSize() ); + QVERIFY( opts.minChunkSize() < defaultOpts.initialChunkSize() ); + QCOMPARE( opts.initialChunkSize(), defaultOpts.initialChunkSize() ); + QCOMPARE( opts.maxChunkSize(), defaultOpts.maxChunkSize() ); + + propagator.account()->setMaxRequestSizeIfLower(defaultOpts.minChunkSize()); + propagator.setSyncOptions(opts); + + opts = propagator.syncOptions(); + QCOMPARE( opts.minChunkSize(), defaultOpts.minChunkSize() ); + QCOMPARE( opts.initialChunkSize(), defaultOpts.minChunkSize() ); + QCOMPARE( opts.maxChunkSize(), defaultOpts.minChunkSize() ); + } }; QTEST_APPLESS_MAIN(TestNextcloudPropagator) diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index 10fcea8e63c30..264afc0135b43 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -9,6 +9,7 @@ #include "caseclashconflictsolver.h" #include "configfile.h" +#include "syncoptions.h" #include "propagatorjobs.h" #include "syncengine.h" @@ -624,6 +625,102 @@ private slots: QCOMPARE(n507, 3); } + /** + * Checks whether a HTTP-413 reply reduces the maxChunkSize in the propagator + */ + void testReplyOfRequestEntityToLargeReducesChunkSize() + { + FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; + + const int minRequestSize = SyncOptions::chunkV2MinChunkSize; + int maxAllowedRequestSize = SyncOptions::maxChunkSizeAfterFailure; + + SyncOptions options; + options.setMinChunkSize(minRequestSize); + options.setMaxChunkSize(4 * maxAllowedRequestSize); + options.setInitialChunkSize(4 * maxAllowedRequestSize); + options._parallelNetworkJobs = 0; + + auto &&engine = fakeFolder.syncEngine(); + engine.setSyncOptions(options); + + // Produce an error based on upload size + QObject parent; + int n413 = 0, nPUT = 0; + fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *outgoingData) -> QNetworkReply * { + FakeErrorReply *reply = nullptr; + if (op == QNetworkAccessManager::PutOperation && outgoingData != nullptr) { + nPUT++; + auto requestSize = outgoingData->size(); + auto closeConnection = getFilePathFromUrl(request.url()).contains("bigClose"); + + if (requestSize > maxAllowedRequestSize) { + if (closeConnection) { + reply = new FakeErrorReply(op, request, &parent, 500); + reply->setError(QNetworkReply::RemoteHostClosedError, ""); + } else { + n413++; + reply = new FakeErrorReply(op, request, &parent, 413); + } + } + } + return reply; + }); + + auto reset = [&]() { + nPUT = n413 = 0; + }; + + // Test initial upload at max size succeeds + reset(); + fakeFolder.localModifier().insert("A/big", maxAllowedRequestSize); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(nPUT, 1); + QCOMPARE(n413, 0); + QCOMPARE(fakeFolder.account()->getMaxRequestSize(), -1); + + // Test SyncOptions::maxChunkSizeAfterFailure applies on first failure + reset(); + fakeFolder.localModifier().insert("A/big1", maxAllowedRequestSize + 1); + QVERIFY(!fakeFolder.syncOnce() && engine.isAnotherSyncNeeded() == AnotherSyncNeeded::ImmediateFollowUp); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(nPUT, 2); + QCOMPARE(n413, 1); + QCOMPARE(fakeFolder.account()->getMaxRequestSize(), SyncOptions::maxChunkSizeAfterFailure); + + // Test "Connection: Close" applies SyncOptions::maxChunkSizeAfterFailure as limit + reset(); + fakeFolder.account()->setMaxRequestSize(-1); + fakeFolder.localModifier().insert("A/bigClose", SyncOptions::maxChunkSizeAfterFailure + 1); + QVERIFY(!fakeFolder.syncOnce() && engine.isAnotherSyncNeeded() == AnotherSyncNeeded::ImmediateFollowUp); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(nPUT, 3); + QCOMPARE(n413, 0); + QCOMPARE(fakeFolder.account()->getMaxRequestSize(), SyncOptions::maxChunkSizeAfterFailure); + + // Test _maxChunkSize is reduced until upload succeeds + reset(); + maxAllowedRequestSize = options.minChunkSize(); + fakeFolder.localModifier().insert("A/big2", options.minChunkSize() * 4); + QVERIFY(!fakeFolder.syncOnce() && engine.isAnotherSyncNeeded() == AnotherSyncNeeded::ImmediateFollowUp); + QVERIFY(!fakeFolder.syncOnce() && engine.isAnotherSyncNeeded() == AnotherSyncNeeded::ImmediateFollowUp); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(nPUT, 4); + QCOMPARE(n413, 2); + QCOMPARE(fakeFolder.account()->getMaxRequestSize(), options.minChunkSize()); + + // Test _maxChunkSize is not reduced below minChunkSize() and retry is not requested + reset(); + maxAllowedRequestSize = options.minChunkSize() / 2; + fakeFolder.localModifier().insert("A/big3", options.minChunkSize()); + QVERIFY(!fakeFolder.syncOnce() && engine.isAnotherSyncNeeded() == AnotherSyncNeeded::NoFollowUpSync); + QVERIFY(!fakeFolder.syncOnce()); + QVERIFY(!fakeFolder.syncOnce()); + QCOMPARE(nPUT, 2); + QCOMPARE(n413, 2); + QCOMPARE(fakeFolder.account()->getMaxRequestSize(), options.minChunkSize()); + } + // Checks whether downloads with bad checksums are accepted void testChecksumValidation() { @@ -850,9 +947,9 @@ private slots: { FakeFolder fakeFolder{ FileInfo{} }; SyncOptions options; - options._initialChunkSize = 10; - options.setMaxChunkSize(10); - options.setMinChunkSize(10); + options.setMinChunkSize(SyncOptions::chunkV2MinChunkSize); + options.setMaxChunkSize(SyncOptions::chunkV2MinChunkSize); + options.setInitialChunkSize(SyncOptions::chunkV2MinChunkSize); fakeFolder.syncEngine().setSyncOptions(options); QObject parent; @@ -865,8 +962,8 @@ private slots: return nullptr; }); - fakeFolder.localModifier().insert("file", 100, 'W'); - QTimer::singleShot(100, &fakeFolder.syncEngine(), [&]() { fakeFolder.syncEngine().abort(); }); + fakeFolder.localModifier().insert("file", 5 * options.maxChunkSize(), 'W'); + QTimer::singleShot(500, &fakeFolder.syncEngine(), [&]() { fakeFolder.syncEngine().abort(); }); QVERIFY(!fakeFolder.syncOnce()); QCOMPARE(nPUT, 3); diff --git a/translations/client_en.ts b/translations/client_en.ts index b19ed6c250080..4202ed9b61b62 100644 --- a/translations/client_en.ts +++ b/translations/client_en.ts @@ -2432,6 +2432,11 @@ It is not advisable to use it. Local file changed during sync. + + + Upload of %1 exceeds the max upload size allowed by the server. Reducing max upload size to %2 + +