Upload: Refactored HTTP-413 handling

* Moved server state to Account
* Added failsafe config option
* Handle connection close if upload-size > failsafe value

Signed-off-by: Juergen Kellerer <juergen@k123.eu>

src/gui/folder.cpp

@@ -912,6 +912,7 @@ void Folder::setSyncOptions()
     opt._initialChunkSize = cfgFile.chunkSize();
     opt._minChunkSize = cfgFile.minChunkSize();
     opt._maxChunkSize = cfgFile.maxChunkSize();
+    opt._failsafeMaxChunkSize = cfgFile.failsafeMaxChunkSize();
     opt._targetChunkUploadDuration = cfgFile.targetChunkUploadDuration();
 
     opt.fillFromEnvironmentVariables();

src/libsync/account.h

@@ -16,6 +16,7 @@
 #ifndef SERVERCONNECTION_H
 #define SERVERCONNECTION_H
 
+#include <QAtomicInteger>
 #include <QByteArray>
 #include <QUrl>
 #include <QNetworkCookie>
@@ -251,6 +252,20 @@ 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() { return _maxRequestSize; }
+    void setMaxRequestSize(qint64 value) { _maxRequestSize = value; }
+    void setMaxRequestSizeIfLower(qint64 value) {
+        while (true) {
+            auto size = _maxRequestSize;
+
+            if (size > 0 && value >= size)
+                break;
+            if (_maxRequestSize.testAndSetOrdered(size, value))
+                break;
+        }
+    }
+
     void clearCookieJar();
     void lendCookieJarTo(QNetworkAccessManager *guest);
     QString cookieJarPath();
@@ -370,6 +385,7 @@ protected Q_SLOTS:
     QSharedPointer<QNetworkAccessManager> _am;
     QScopedPointer<AbstractCredentials> _credentials;
     bool _http2Supported = false;
+    QAtomicInteger<qint64> _maxRequestSize = -1;
 
     /// Certificates that were explicitly rejected by the user
     QList<QSslCertificate> _rejectedCertificates;

src/libsync/configfile.cpp

@@ -80,6 +80,7 @@ static const char timeoutC[] = "timeout";
 static const char chunkSizeC[] = "chunkSize";
 static const char minChunkSizeC[] = "minChunkSize";
 static const char maxChunkSizeC[] = "maxChunkSize";
+static const char failsafeMaxChunkSizeC[] = "failsafeMaxChunkSize";
 static const char targetChunkUploadDurationC[] = "targetChunkUploadDuration";
 static const char automaticLogDirC[] = "logToTemporaryLogDir";
 static const char logDirC[] = "logDir";
@@ -245,6 +246,12 @@ qint64 ConfigFile::maxChunkSize() const
     return settings.value(QLatin1String(maxChunkSizeC), 1000 * 1000 * 1000).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
+}
+
 qint64 ConfigFile::minChunkSize() const
 {
     QSettings settings(configFile(), QSettings::IniFormat);

src/libsync/configfile.h

@@ -161,6 +161,7 @@ class OWNCLOUDSYNC_EXPORT ConfigFile
     int timeout() const;
     qint64 chunkSize() const;
     qint64 maxChunkSize() const;
+    qint64 failsafeMaxChunkSize() const;
     qint64 minChunkSize() const;
     std::chrono::milliseconds targetChunkUploadDuration() const;
 

src/libsync/owncloudpropagator.cpp

@@ -673,7 +673,7 @@ const SyncOptions &OwncloudPropagator::syncOptions() const
 void OwncloudPropagator::setSyncOptions(const SyncOptions &syncOptions)
 {
     _syncOptions = syncOptions;
-    _syncOptions.verifyChunkSizes();
+    _syncOptions.verifyChunkSizes(account()->getMaxRequestSize());
     _chunkSize = _syncOptions._initialChunkSize;
 }
 

src/libsync/propagateupload.cpp

@@ -676,28 +676,41 @@ 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 chunkSize = _item->_requestBodySize;
+        auto requestSize = _item->_requestBodySize;
 
-        // Set new bound as global limit (we need to guess here, as sent bytes are no good indicator what is allowed)
-        auto maxChunkSize = chunkSize / 2 + qMax(qint64(0), opts._minChunkSize) / 2;
-        SyncOptions::limitMaxChunkSize(maxChunkSize);
-        opts.verifyChunkSizes();
+        // 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
+        // Apply to this propagator (limit is applied in setSyncOptions())
         propagator()->setSyncOptions(opts);
         opts = propagator()->syncOptions();
 
         // Trigger retry
-        if (opts._maxChunkSize < chunkSize) {
+        if (opts._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(chunkSize))
+                        .arg(Utility::octetsToString(requestSize))
                         .arg(Utility::octetsToString(opts._maxChunkSize));
     }
 

src/libsync/syncoptions.cpp

@@ -20,10 +20,7 @@
 using namespace OCC;
 
 // limit cannot be lower than 512KB
-const qint64 lowestMaxChunkSizeLimit = 512 * 1024;
-
-// initial limit -1 (no limit)
-QAtomicInteger<qint64> SyncOptions::_maxChunkSizeLimit = -1;
+const qint64 lowestPossibleChunkSize = 512 * 1024;
 
 SyncOptions::SyncOptions()
     : _vfs(new VfsOff)
@@ -46,6 +43,10 @@ void SyncOptions::fillFromEnvironmentVariables()
     if (!maxChunkSizeEnv.isEmpty())
         _maxChunkSize = maxChunkSizeEnv.toUInt();
 
+    QByteArray failsafeMaxChunkSizeEnv = qgetenv("OWNCLOUD_FAILSAFE_MAX_CHUNK_SIZE");
+    if (!failsafeMaxChunkSizeEnv.isEmpty())
+        _failsafeMaxChunkSize = failsafeMaxChunkSizeEnv.toUInt();
+
     QByteArray targetChunkUploadDurationEnv = qgetenv("OWNCLOUD_TARGET_CHUNK_UPLOAD_DURATION");
     if (!targetChunkUploadDurationEnv.isEmpty())
         _targetChunkUploadDuration = std::chrono::milliseconds(targetChunkUploadDurationEnv.toUInt());
@@ -55,37 +56,23 @@ void SyncOptions::fillFromEnvironmentVariables()
         _parallelNetworkJobs = maxParallel;
 }
 
-void SyncOptions::verifyChunkSizes()
+void SyncOptions::verifyChunkSizes(qint64 maxUpperLimit)
 {
-    qint64 maxUpperLimit = _maxChunkSizeLimit;
     if (maxUpperLimit > 0) {
         _initialChunkSize = qMin(maxUpperLimit, _initialChunkSize);
         _minChunkSize = qMin(maxUpperLimit, _minChunkSize);
         _maxChunkSize = qMin(maxUpperLimit, _maxChunkSize);
     }
 
+    _initialChunkSize = qMax(lowestPossibleChunkSize, _initialChunkSize);
+    _minChunkSize = qMax(lowestPossibleChunkSize, _minChunkSize);
+    _maxChunkSize = qMax(lowestPossibleChunkSize, _maxChunkSize);
+    _failsafeMaxChunkSize = qMax(lowestPossibleChunkSize, _failsafeMaxChunkSize);
+
     _minChunkSize = qMin(_minChunkSize, _initialChunkSize);
     _maxChunkSize = qMax(_maxChunkSize, _initialChunkSize);
 }
 
-void SyncOptions::limitMaxChunkSize(qint64 maxChunkSizeLimit)
-{
-    if (maxChunkSizeLimit < 0) {
-        // Disable the limit
-        _maxChunkSizeLimit = -1;
-    } else {
-        // Lower the limit concurrently (but never increase)
-        auto newLimit = qMax(lowestMaxChunkSizeLimit, maxChunkSizeLimit);
-
-        for (auto limit = _maxChunkSizeLimit; ; limit = _maxChunkSizeLimit) {
-            if (newLimit >= limit && limit > 0)
-                break;
-            if (_maxChunkSizeLimit.testAndSetOrdered(limit, newLimit))
-                break;
-        }
-    }
-}
-
 QRegularExpression SyncOptions::fileRegex() const
 {
     return _fileRegex;

src/libsync/syncoptions.h

@@ -17,7 +17,6 @@
 #include "owncloudlib.h"
 #include "common/vfs.h"
 
-#include <QAtomicInteger>
 #include <QRegularExpression>
 #include <QSharedPointer>
 #include <QString>
@@ -64,6 +63,11 @@ class OWNCLOUDSYNC_EXPORT SyncOptions
     /** The maximum chunk size in bytes for chunked uploads */
     qint64 _maxChunkSize = 1000 * 1000 * 1000; // 1000MB
 
+    /** 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
+
     /** The target duration of chunk uploads for dynamic chunk sizing.
      *
      * Set to 0 it will disable dynamic chunk sizing.
@@ -87,10 +91,7 @@ class OWNCLOUDSYNC_EXPORT SyncOptions
      * values. To cope with this, adjust min/max to always include the
      * initial chunk size value.
      */
-    void verifyChunkSizes();
-
-    /** Limits the max values of _minChunkSize and _maxChunkSize, set to -1 (default) to disable the limit */
-    static void limitMaxChunkSize(qint64 maxChunkSizeLimit);
+    void verifyChunkSizes(qint64 maxUpperLimit = -1);
 
     /** A regular expression to match file names
      * If no pattern is provided the default is an invalid regular expression.
@@ -108,11 +109,6 @@ class OWNCLOUDSYNC_EXPORT SyncOptions
     void setPathPattern(const QString &pattern);
 
 private:
-    /** Global max value for _minChunkSize & _maxChunkSize
-     * Only applied in verifyChunkSizes() if > 0
-     * */
-    static QAtomicInteger<qint64> _maxChunkSizeLimit;
-
     /**
      * Only sync files that mathc the expression
      * Invalid pattern by default.