From c830cfab4f9b264c26c0111fe923562b82f72fed Mon Sep 17 00:00:00 2001 From: Matt Broadstone Date: Mon, 18 Aug 2014 17:02:52 -0400 Subject: 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 --- src/network/access/qnetworkaccessftpbackend.cpp | 20 +++++++++--------- src/network/access/qnetworkaccessftpbackend_p.h | 7 ++++++- tests/auto/qnetworkreply/tst_qnetworkreply.cpp | 27 +++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/network/access/qnetworkaccessftpbackend.cpp b/src/network/access/qnetworkaccessftpbackend.cpp index 449a51e..541f8f4 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 e64e2b4..ea5cda3 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 5e295cb..ddf4b17 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("referenceName"); -- cgit v0.12