diff --git a/src/libsync/account.h b/src/libsync/account.h index a209210c32f2a..fd7283c31576a 100644 --- a/src/libsync/account.h +++ b/src/libsync/account.h @@ -253,11 +253,11 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject void setHttp2Supported(bool value) { _http2Supported = value; } /** Max allowed request size in bytes that the server accepts. Is <= 0 if not limited */ - qint64 getMaxRequestSize() { return _maxRequestSize; } + qint64 getMaxRequestSize() const { return _maxRequestSize; } void setMaxRequestSize(qint64 value) { _maxRequestSize = value; } void setMaxRequestSizeIfLower(qint64 value) { while (true) { - auto size = _maxRequestSize; + const auto size = _maxRequestSize; if (size > 0 && value >= size) break; diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp index 8d053f073d7bf..a6533be8dc4aa 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" @@ -237,25 +238,25 @@ int ConfigFile::timeout() const qint64 ConfigFile::chunkSize() const { QSettings settings(configFile(), QSettings::IniFormat); - return settings.value(QLatin1String(chunkSizeC), 10 * 1000 * 1000).toLongLong(); // default to 10 MB + return settings.value(QLatin1String(chunkSizeC), DefaultInitialChunkSize).toLongLong(); // default to 10 MB } qint64 ConfigFile::maxChunkSize() const { QSettings settings(configFile(), QSettings::IniFormat); - return settings.value(QLatin1String(maxChunkSizeC), 1000 * 1000 * 1000).toLongLong(); // default to 1000 MB + return settings.value(QLatin1String(maxChunkSizeC), DefaultMaxChunkSize).toLongLong(); // default to 1000 MB } qint64 ConfigFile::failsafeMaxChunkSize() const { QSettings settings(configFile(), QSettings::IniFormat); - return settings.value(QLatin1String(failsafeMaxChunkSizeC), 100 * 1000 * 1000).toLongLong(); // default to 100 MB + return settings.value(QLatin1String(failsafeMaxChunkSizeC), DefaultFailsafeMaxChunkSize).toLongLong(); // default to 100 MB } qint64 ConfigFile::minChunkSize() const { QSettings settings(configFile(), QSettings::IniFormat); - return settings.value(QLatin1String(minChunkSizeC), 1000 * 1000).toLongLong(); // default to 1 MB + return settings.value(QLatin1String(minChunkSizeC), DefaultMinChunkSize).toLongLong(); // default to 1 MB } chrono::milliseconds ConfigFile::targetChunkUploadDuration() const diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index 832b5fbe8e455..d5a031df5f430 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -676,42 +676,45 @@ void PropagateUploadFileCommon::commonErrorHandling(AbstractNetworkJob *job) SyncFileItem::Status status = classifyError(job->reply()->error(), _item->_httpErrorCode, &propagator()->_anotherSyncNeeded, replyContent); - // Remote host closed the connection after an upload larger than _failsafeMaxChunkSize - if (job->reply()->error() == QNetworkReply::RemoteHostClosedError - && _item->_requestBodySize > propagator()->syncOptions()._failsafeMaxChunkSize) { - // treat it as if the server returned HTTP-413 - qCInfo(lcPropagateUpload()) << "Remote host closed the connection when uploading" - << Utility::octetsToString(_item->_requestBodySize) << "." - << "Treating the error like HTTP-413 (request entity to large)"; - - _item->_httpErrorCode = 413; - } - // Request entity to large: Chunk size is larger than supported by the server - if (_item->_httpErrorCode == 413 && _item->_requestBodySize > 0 && status == SyncFileItem::NormalError) { - auto opts = propagator()->syncOptions(); - auto requestSize = _item->_requestBodySize; - - // Set new bound as account upload limit (we need to guess here, as sent bytes are no good indicator what is allowed) - auto maxRequestSize = requestSize > opts._failsafeMaxChunkSize - ? opts._failsafeMaxChunkSize - : qMax(opts._minChunkSize, requestSize / 2); - - propagator()->account()->setMaxRequestSizeIfLower(maxRequestSize); - - // Apply to this propagator (limit is applied in setSyncOptions()) - propagator()->setSyncOptions(opts); - opts = propagator()->syncOptions(); - - // Trigger retry - if (opts._maxChunkSize < requestSize) { - propagator()->_anotherSyncNeeded = true; - status = SyncFileItem::SoftError; + 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 _failsafeMaxChunkSize + const bool isConnectionClosedOnLargeUpload = job->reply()->error() == QNetworkReply::RemoteHostClosedError + && requestSize > opts._failsafeMaxChunkSize; + + if (isConnectionClosedOnLargeUpload) { + qCInfo(lcPropagateUpload()) << "Remote host closed the connection when uploading" + << Utility::octetsToString(requestSize) << "." + << "Treating the error like HTTP-413 (request entity to large)"; } - 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(opts._maxChunkSize)); + // 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 = requestSize > opts._failsafeMaxChunkSize + ? opts._failsafeMaxChunkSize + : qMax(opts._minChunkSize, requestSize / 2); + + 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. diff --git a/src/libsync/syncoptions.cpp b/src/libsync/syncoptions.cpp index ef9aaeb1a6323..2d95acade104e 100644 --- a/src/libsync/syncoptions.cpp +++ b/src/libsync/syncoptions.cpp @@ -19,9 +19,6 @@ using namespace OCC; -// limit cannot be lower than 512KB -const qint64 lowestPossibleChunkSize = 512 * 1024; - SyncOptions::SyncOptions() : _vfs(new VfsOff) { @@ -64,10 +61,10 @@ void SyncOptions::verifyChunkSizes(qint64 maxUpperLimit) _maxChunkSize = qMin(maxUpperLimit, _maxChunkSize); } - _initialChunkSize = qMax(lowestPossibleChunkSize, _initialChunkSize); - _minChunkSize = qMax(lowestPossibleChunkSize, _minChunkSize); - _maxChunkSize = qMax(lowestPossibleChunkSize, _maxChunkSize); - _failsafeMaxChunkSize = qMax(lowestPossibleChunkSize, _failsafeMaxChunkSize); + _initialChunkSize = qMax(LowestPossibleChunkSize, _initialChunkSize); + _minChunkSize = qMax(LowestPossibleChunkSize, _minChunkSize); + _maxChunkSize = qMax(LowestPossibleChunkSize, _maxChunkSize); + _failsafeMaxChunkSize = qMax(LowestPossibleChunkSize, _failsafeMaxChunkSize); _minChunkSize = qMin(_minChunkSize, _initialChunkSize); _maxChunkSize = qMax(_maxChunkSize, _initialChunkSize); diff --git a/src/libsync/syncoptions.h b/src/libsync/syncoptions.h index 710a62acd9583..0640138fa64de 100644 --- a/src/libsync/syncoptions.h +++ b/src/libsync/syncoptions.h @@ -26,6 +26,21 @@ namespace OCC { +/** Default value for SyncOptions::_initialChunkSize ( 10MB) */ +const qint64 DefaultInitialChunkSize = 10 * 1000 * 1000; + +/** Default value for SyncOptions::_minChunkSize ( 1MB) */ +const qint64 DefaultMinChunkSize = 1 * 1000 * 1000; + +/** Default value for SyncOptions::_maxChunkSize (1000MB) */ +const qint64 DefaultMaxChunkSize = 1000 * 1000 * 1000; + +/** Default value for SyncOptions::_failsafeMaxChunkSize ( 100MB) */ +const qint64 DefaultFailsafeMaxChunkSize = 100 * 1000 * 1000; + +/** No chunk size can be set to a value lower than this ( 512KB) */ +const qint64 LowestPossibleChunkSize = 512 * 1000; + /** * Value class containing the options given to the sync engine */ @@ -55,18 +70,18 @@ class OWNCLOUDSYNC_EXPORT SyncOptions * starting value and is then gradually adjusted within the * minChunkSize / maxChunkSize bounds. */ - qint64 _initialChunkSize = 10 * 1000 * 1000; // 10MB + qint64 _initialChunkSize = DefaultInitialChunkSize; /** The minimum chunk size in bytes for chunked uploads */ - qint64 _minChunkSize = 1 * 1000 * 1000; // 1MB + qint64 _minChunkSize = DefaultMinChunkSize; /** The maximum chunk size in bytes for chunked uploads */ - qint64 _maxChunkSize = 1000 * 1000 * 1000; // 1000MB + qint64 _maxChunkSize = DefaultMaxChunkSize; /** Failsafe configuration of the maximum chunk size in bytes * Applies when a chunked upload caused HTTP-413 or RemoteHostClosedError */ - qint64 _failsafeMaxChunkSize = 100 * 1000 * 1000; // 100MB + qint64 _failsafeMaxChunkSize = DefaultFailsafeMaxChunkSize; /** The target duration of chunk uploads for dynamic chunk sizing. * diff --git a/test/testnextcloudpropagator.cpp b/test/testnextcloudpropagator.cpp index c66f49030d337..5488a8d92a8bc 100644 --- a/test/testnextcloudpropagator.cpp +++ b/test/testnextcloudpropagator.cpp @@ -10,6 +10,7 @@ #include "account.h" #include "propagatedownload.h" #include "owncloudpropagator_p.h" +#include "syncoptions.h" using namespace OCC; namespace OCC { @@ -81,7 +82,7 @@ private slots: void testRespectsLowestPossibleChunkSize() { QSet __blacklist; - OwncloudPropagator propagator(OCC::Account::create(), "", "", nullptr, __blacklist); + OwncloudPropagator propagator(Account::create(), "", "", nullptr, __blacklist); auto opts = propagator.syncOptions(); opts._minChunkSize = @@ -91,15 +92,15 @@ private slots: propagator.setSyncOptions(opts); opts = propagator.syncOptions(); - QCOMPARE( opts._minChunkSize, 512 * 1024 ); - QCOMPARE( opts._initialChunkSize, 512 * 1024 ); - QCOMPARE( opts._maxChunkSize, 512 * 1024 ); + QCOMPARE( opts._minChunkSize, LowestPossibleChunkSize ); + QCOMPARE( opts._initialChunkSize, LowestPossibleChunkSize ); + QCOMPARE( opts._maxChunkSize, LowestPossibleChunkSize ); } void testLimitsMaxChunkSizeByAccount() { QSet __blacklist; - OwncloudPropagator propagator(OCC::Account::create(), "", "", nullptr, __blacklist); + OwncloudPropagator propagator(Account::create(), "", "", nullptr, __blacklist); auto opts = propagator.syncOptions(); SyncOptions defaultOpts; diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index 96231abf98b51..ce3065a6adf2d 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -8,6 +8,7 @@ #include #include "syncenginetestutils.h" #include +#include #include using namespace OCC; @@ -599,7 +600,7 @@ private slots: { FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; - const int minRequestSize = 512 * 1024; + const int minRequestSize = LowestPossibleChunkSize; int maxAllowedRequestSize = 4 * minRequestSize; SyncOptions options; @@ -911,7 +912,7 @@ private slots: SyncOptions options; options._initialChunkSize = options._maxChunkSize = - options._minChunkSize = 1000 * 1000; + options._minChunkSize = LowestPossibleChunkSize; fakeFolder.syncEngine().setSyncOptions(options);