From 100ed2e91ead45f59639ec6b8aad16e6752818c4 Mon Sep 17 00:00:00 2001
From: Peter Hartmann <phartmann@blackberry.com>
Date: Tue, 24 Jun 2014 14:49:51 +0200
Subject: [PATCH] network internals: do not try to cache a deleted entry

We were keeping a dangling pointer to a non-existent QIODevice around
which would lead to a crash.

Task-number: QTBUG-17400
Change-Id: Ie374cbb94bb45c9b0fbef46287b3317f60154123
Reviewed-by: Richard J. Moore <rich@kde.org>
---
 src/network/access/qnetworkreplyhttpimpl.cpp  | 10 +++++
 src/network/access/qnetworkreplyhttpimpl_p.h  |  3 ++
 .../tst_qnetworkdiskcache.cpp                 | 38 +++++++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp
index e136eee5ca..56105a544b 100644
--- a/src/network/access/qnetworkreplyhttpimpl.cpp
+++ b/src/network/access/qnetworkreplyhttpimpl.cpp
@@ -975,6 +975,9 @@ void QNetworkReplyHttpImplPrivate::initCacheSaveDevice()
 
     cacheSaveDevice = managerPrivate->networkCache->prepare(metaData);
 
+    if (cacheSaveDevice)
+        q->connect(cacheSaveDevice, SIGNAL(aboutToClose()), SLOT(_q_cacheSaveDeviceAboutToClose()));
+
     if (!cacheSaveDevice || (cacheSaveDevice && !cacheSaveDevice->isOpen())) {
         if (cacheSaveDevice && !cacheSaveDevice->isOpen())
             qCritical("QNetworkReplyImpl: network cache returned a device that is not open -- "
@@ -1708,6 +1711,13 @@ void QNetworkReplyHttpImplPrivate::_q_bufferOutgoingDataFinished()
     QMetaObject::invokeMethod(q, "_q_startOperation", Qt::QueuedConnection);
 }
 
+void QNetworkReplyHttpImplPrivate::_q_cacheSaveDeviceAboutToClose()
+{
+    // do not keep a dangling pointer to the device around (device
+    // is closing because e.g. QAbstractNetworkCache::remove() was called).
+    cacheSaveDevice = 0;
+}
+
 void QNetworkReplyHttpImplPrivate::_q_bufferOutgoingData()
 {
     Q_Q(QNetworkReplyHttpImpl);
diff --git a/src/network/access/qnetworkreplyhttpimpl_p.h b/src/network/access/qnetworkreplyhttpimpl_p.h
index aa2d6f0ec9..d21659ce82 100644
--- a/src/network/access/qnetworkreplyhttpimpl_p.h
+++ b/src/network/access/qnetworkreplyhttpimpl_p.h
@@ -130,6 +130,7 @@ public:
     Q_PRIVATE_SLOT(d_func(), void wantUploadDataSlot(qint64))
     Q_PRIVATE_SLOT(d_func(), void sentUploadDataSlot(qint64))
     Q_PRIVATE_SLOT(d_func(), void emitReplyUploadProgress(qint64, qint64))
+    Q_PRIVATE_SLOT(d_func(), void _q_cacheSaveDeviceAboutToClose())
 
 
 #ifndef QT_NO_SSL
@@ -179,6 +180,8 @@ public:
     void _q_bufferOutgoingData();
     void _q_bufferOutgoingDataFinished();
 
+    void _q_cacheSaveDeviceAboutToClose();
+
 #ifndef QT_NO_BEARERMANAGEMENT
     void _q_networkSessionConnected();
     void _q_networkSessionFailed();
diff --git a/tests/auto/network/access/qnetworkdiskcache/tst_qnetworkdiskcache.cpp b/tests/auto/network/access/qnetworkdiskcache/tst_qnetworkdiskcache.cpp
index 44edcc66c0..4740b92b84 100644
--- a/tests/auto/network/access/qnetworkdiskcache/tst_qnetworkdiskcache.cpp
+++ b/tests/auto/network/access/qnetworkdiskcache/tst_qnetworkdiskcache.cpp
@@ -62,6 +62,7 @@ public slots:
     void cleanupTestCase();
     void init();
     void cleanup();
+    void accessAfterRemoveReadyReadSlot();
 
 private slots:
     void qnetworkdiskcache_data();
@@ -74,6 +75,7 @@ private slots:
     void data();
     void metaData();
     void remove();
+    void accessAfterRemove(); // QTBUG-17400
     void setCacheDirectory_data();
     void setCacheDirectory();
     void updateMetaData();
@@ -89,6 +91,8 @@ private slots:
 
 private:
     QTemporaryDir tempDir;
+    QUrl url; // used by accessAfterRemove()
+    QNetworkDiskCache *diskCache; // used by accessAfterRemove()
 };
 
 // FIXME same as in tst_qnetworkreply.cpp .. could be unified
@@ -370,6 +374,40 @@ void tst_QNetworkDiskCache::remove()
     QCOMPARE(countFiles(cacheDirectory).count(), NUM_SUBDIRECTORIES + 2);
 }
 
+void tst_QNetworkDiskCache::accessAfterRemove() // QTBUG-17400
+{
+    QByteArray data("HTTP/1.1 200 OK\r\n"
+                    "Content-Length: 1\r\n"
+                    "\r\n"
+                    "a");
+
+    MiniHttpServer server(data);
+
+    QNetworkAccessManager *manager = new QNetworkAccessManager();
+    SubQNetworkDiskCache subCache;
+    subCache.setCacheDirectory(QLatin1String("cacheDir"));
+    diskCache = &subCache;
+    manager->setCache(&subCache);
+
+    url = QUrl("http://127.0.0.1:" + QString::number(server.serverPort()));
+    QNetworkRequest request(url);
+
+    QNetworkReply *reply = manager->get(request);
+    connect(reply, SIGNAL(readyRead()), this, SLOT(accessAfterRemoveReadyReadSlot()));
+    connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop()));
+
+    QTestEventLoop::instance().enterLoop(5);
+    QVERIFY(!QTestEventLoop::instance().timeout());
+
+    reply->deleteLater();
+    manager->deleteLater();
+}
+
+void tst_QNetworkDiskCache::accessAfterRemoveReadyReadSlot()
+{
+    diskCache->remove(url); // this used to cause a crash later on
+}
+
 void tst_QNetworkDiskCache::setCacheDirectory_data()
 {
     QTest::addColumn<QString>("cacheDir");