refactor disconnectFromFtp to remove cached entries when necessary

In cases where a cached ftp connection fails to connect, or a file
transfer has failed, we should removed the cached connection. Since qnam
has an idea of a single internal QFtp per full operation, when file
transfers failed previously the cached connection would be reused for
subsequent connections and thus fail.

[ChangeLog][QtNetwork][QNetworkAccessManager] QNetworkAccessManager now
properly handles FTP transfer failures by removing failed cached ftp
connections.

Task-number: QTBUG-40797
(cherry picked and adapted from qtbase/45cbbe56bc13216b83215ea148590eccf81f420a)
Change-Id: Ie9eec5ec54af16a8d19e34d04bdd993cc7bbd0f5
Reviewed-by: Richard J. Moore <rich@kde.org>

3 files changed, 42 insertions, 12 deletions

diff --git a/src/network/access/qnetworkaccessftpbackend.cpp b/src/network/access/qnetworkaccessftpbackend.cpp
index 449a51e618..541f8f426f 100644
--- a/src/network/access/qnetworkaccessftpbackend.cpp
+++ b/src/network/access/qnetworkaccessftpbackend.cpp
@@ -205,7 +205,7 @@ void QNetworkAccessFtpBackend::ftpConnectionReady(QNetworkAccessCache::Cacheable
     // no, defer the actual operation until after we've logged in
 }
 
-void QNetworkAccessFtpBackend::disconnectFromFtp()
+void QNetworkAccessFtpBackend::disconnectFromFtp(CacheCleanupMode mode)
 {
     state = Disconnecting;
 
@@ -213,7 +213,12 @@ void QNetworkAccessFtpBackend::disconnectFromFtp()
         disconnect(ftp, 0, this, 0);
 
         QByteArray key = makeCacheKey(url());
-        QNetworkAccessManagerPrivate::getObjectCache(this)->releaseEntry(key);
+        if (mode == RemoveCachedConnection) {
+            QNetworkAccessManagerPrivate::getObjectCache(this)->removeEntry(key);
+            ftp->dispose();
+        } else {
+            QNetworkAccessManagerPrivate::getObjectCache(this)->releaseEntry(key);
+        }
 
         ftp = 0;
     }
@@ -263,14 +268,7 @@ void QNetworkAccessFtpBackend::ftpDone()
         }
 
         // we're not connected, so remove the cache entry:
-        QByteArray key = makeCacheKey(url());
-        QNetworkAccessManagerPrivate::getObjectCache(this)->removeEntry(key);
-
-        disconnect(ftp, 0, this, 0);
-        ftp->dispose();
-        ftp = 0;
-
-        state = Disconnecting;
+        disconnectFromFtp(RemoveCachedConnection);
         finished();
         return;
     }
@@ -290,7 +288,7 @@ void QNetworkAccessFtpBackend::ftpDone()
         else
             error(QNetworkReply::ContentAccessDenied, msg);
 
-        disconnectFromFtp();
+        disconnectFromFtp(RemoveCachedConnection);
         finished();
     }
 
diff --git a/src/network/access/qnetworkaccessftpbackend_p.h b/src/network/access/qnetworkaccessftpbackend_p.h
index e64e2b4222..ea5cda39e4 100644
--- a/src/network/access/qnetworkaccessftpbackend_p.h
+++ b/src/network/access/qnetworkaccessftpbackend_p.h
@@ -90,7 +90,12 @@ public:
 
     virtual void downstreamReadyWrite();
 
-    void disconnectFromFtp();
+    enum CacheCleanupMode {
+        ReleaseCachedConnection,
+        RemoveCachedConnection
+    };
+
+    void disconnectFromFtp(CacheCleanupMode mode = ReleaseCachedConnection);
 
 public slots:
     void ftpConnectionReady(QNetworkAccessCache::CacheableObject *object);
diff --git a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp
index 5e295cba2d..ddf4b17dee 100644
--- a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp
+++ b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp
@@ -194,6 +194,7 @@ private Q_SLOTS:
     void getFromFileSpecial();
     void getFromFtp_data();
     void getFromFtp();
+    void getFromFtpAfterError();    // QTBUG-40797
     void getFromHttp_data();
     void getFromHttp();
     void getErrors_data();
@@ -1598,6 +1599,32 @@ void tst_QNetworkReply::getFromFtp()
     QCOMPARE(reply->readAll(), reference.readAll());
 }
 
+void tst_QNetworkReply::getFromFtpAfterError()
+{
+    QNetworkRequest invalidRequest(QUrl("ftp://" + QtNetworkSettings::serverName() + "/qtest/invalid.txt"));
+    QNetworkReplyPtr invalidReply;
+    invalidReply = manager.get(invalidRequest);
+    QSignalSpy spy(invalidReply.data(), SIGNAL(error(QNetworkReply::NetworkError)));
+
+    // now run the request:
+    connect(invalidReply.data(), SIGNAL(error(QNetworkReply::NetworkError)),
+            &QTestEventLoop::instance(), SLOT(exitLoop()));
+    QTestEventLoop::instance().enterLoop(5);
+    QVERIFY(!QTestEventLoop::instance().timeout());
+    QVERIFY(!spy.isEmpty());
+    QCOMPARE(invalidReply->error(), QNetworkReply::ContentNotFoundError);
+
+    QFile reference("srcdir:/rfc3252.txt");
+    QVERIFY(reference.open(QIODevice::ReadOnly));
+    QNetworkRequest validRequest(QUrl("ftp://" + QtNetworkSettings::serverName() + "/qtest/rfc3252.txt"));
+    QNetworkReplyPtr validReply;
+    RUN_REQUEST(runSimpleRequest(QNetworkAccessManager::GetOperation, validRequest, validReply));
+    QCOMPARE(validReply->url(), validRequest.url());
+    QCOMPARE(validReply->error(), QNetworkReply::NoError);
+    QCOMPARE(validReply->header(QNetworkRequest::ContentLengthHeader).toLongLong(), reference.size());
+    QCOMPARE(validReply->readAll(), reference.readAll());
+}
+
 void tst_QNetworkReply::getFromHttp_data()
 {
     QTest::addColumn<QString>("referenceName");