Work around the Qt PUT corruption bug with Qt < 5.4.2 #2425

Since QNonContiguousByteDeviceThreadForwardImpl::reset will call UploadDevice::reset with a BlockingQueuedConnection, this allows us to reset the HTTP channel along with its buffers before they get the chance to be reused with a subsequent request.
owncloud · May 6, 2015 · a6500d8 · a6500d8
1 parent 0d5d2c5
commit a6500d8
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 0 deletions.
diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp
@@ -32,6 +32,12 @@
 #include "propagator_legacy.h"
 #endif
 
+#if QT_VERSION < QT_VERSION_CHECK(5, 4, 2)
+namespace {
+const char owncloudShouldSoftCancelPropertyName[] = "owncloud-should-soft-cancel";
+}
+#endif
+
 namespace OCC {
 
 /**
@@ -86,6 +92,16 @@ void PUTFileJob::start() {
     connect(reply(), SIGNAL(uploadProgress(qint64,qint64)), this, SIGNAL(uploadProgress(qint64,qint64)));
     connect(this, SIGNAL(networkActivity()), account().data(), SIGNAL(propagatorNetworkActivity()));
 
+    // For Qt versions not including https://codereview.qt-project.org/110150
+    // Also do the runtime check if compiled with an old Qt but running with fixed one.
+#if QT_VERSION < QT_VERSION_CHECK(4, 8, 7)
+    if (QLatin1String(qVersion()) < QLatin1String("4.8.7"))
+        connect(_device.data(), SIGNAL(wasReset()), this, SLOT(slotSoftAbort()));
+#elif QT_VERSION > QT_VERSION_CHECK(5, 0, 0) && QT_VERSION < QT_VERSION_CHECK(5, 4, 2)
+    if (QLatin1String(qVersion()) < QLatin1String("5.4.2"))
+        connect(_device.data(), SIGNAL(wasReset()), this, SLOT(slotSoftAbort()));
+#endif
+
     AbstractNetworkJob::start();
 }
 
@@ -94,6 +110,13 @@ void PUTFileJob::slotTimeout() {
     reply()->abort();
 }
 
+#if QT_VERSION < QT_VERSION_CHECK(5, 4, 2)
+void PUTFileJob::slotSoftAbort() {
+    reply()->setProperty(owncloudShouldSoftCancelPropertyName, true);
+    reply()->abort();
+}
+#endif
+
 void PollJob::start()
 {
     setTimeout(120 * 1000);
@@ -471,6 +494,18 @@ void PropagateUploadFileQNAM::slotPutFinished()
     }
 
     QNetworkReply::NetworkError err = job->reply()->error();
+
+#if QT_VERSION < QT_VERSION_CHECK(5, 4, 2)
+    if (err == QNetworkReply::OperationCanceledError && job->reply()->property(owncloudShouldSoftCancelPropertyName).isValid()) {
+        // Abort the job and try again later.
+        // This works around a bug in QNAM wich might reuse a non-empty buffer for the next request.
+        qDebug() << "Forcing job abort on HTTP connection reset with Qt < 5.4.2.";
+        _propagator->_anotherSyncNeeded = true;
+        done(SyncFileItem::SoftError, tr("Forcing job abort on HTTP connection reset with Qt < 5.4.2."));
+        return;
+    }
+#endif
+
     if (err != QNetworkReply::NoError) {
         _item._httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
         if(checkForProblemsWithShared(_item._httpErrorCode,

diff --git a/src/libsync/propagateupload.h b/src/libsync/propagateupload.h
@@ -40,11 +40,21 @@ class UploadDevice : public QIODevice {
     bool isSequential() const Q_DECL_OVERRIDE;
     bool seek ( qint64 pos ) Q_DECL_OVERRIDE;
 
+#if QT_VERSION < QT_VERSION_CHECK(5, 4, 2)
+    bool reset() Q_DECL_OVERRIDE { emit wasReset(); return QIODevice::reset(); }
+#endif
+
     void setBandwidthLimited(bool);
     bool isBandwidthLimited() { return _bandwidthLimited; }
     void setChoked(bool);
     bool isChoked() { return _choked; }
     void giveBandwidthQuota(qint64 bwq);
+
+signals:
+#if QT_VERSION < QT_VERSION_CHECK(5, 4, 2)
+    void wasReset();
+#endif
+
 private:
 
     // The file data
@@ -95,6 +105,11 @@ class PUTFileJob : public AbstractNetworkJob {
 signals:
     void finishedSignal();
     void uploadProgress(qint64,qint64);
+
+private slots:
+#if QT_VERSION < QT_VERSION_CHECK(5, 4, 2)
+    void slotSoftAbort();
+#endif
 };
 
 /**