From 483a069415f2648a0d4c1f86d69f372607750ae6 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Thu, 17 Mar 2011 14:51:10 +0100 Subject: Fix disk cache interaction for range retrieval HTTP requests. The disk cache API does not currently support retrieving partial content, it can only serve entire files. Therefore we disable the use of the cache for these kinds of requests, as indicated by the presence of the Range header field. Done-with: Jocelyn Turcotte Reviewed-by: Markus Goetz Reviewed-by: Peter Hartmann --- src/network/access/qnetworkaccesshttpbackend.cpp | 5 ++ tests/auto/qnetworkreply/tst_qnetworkreply.cpp | 74 +++++++++++++++++------- 2 files changed, 58 insertions(+), 21 deletions(-) diff --git a/src/network/access/qnetworkaccesshttpbackend.cpp b/src/network/access/qnetworkaccesshttpbackend.cpp index 4189743..108ac68 100644 --- a/src/network/access/qnetworkaccesshttpbackend.cpp +++ b/src/network/access/qnetworkaccesshttpbackend.cpp @@ -360,6 +360,11 @@ void QNetworkAccessHttpBackend::validateCache(QHttpNetworkRequest &httpRequest, return; } + // The disk cache API does not currently support partial content retrieval. + // That is why we don't use the disk cache for any such requests. + if (request().hasRawHeader("Range")) + return; + QAbstractNetworkCache *nc = networkCache(); if (!nc) return; // no local cache diff --git a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp index 39bb0fc..4338e74 100644 --- a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp @@ -2663,6 +2663,7 @@ void tst_QNetworkReply::ioGetFromHttpWithCache_data() QTest::addColumn("body"); QTest::addColumn("cachedReply"); QTest::addColumn("cacheMode"); + QTest::addColumn("extraHttpHeaders"); QTest::addColumn("loadedFromCache"); QTest::addColumn("networkUsed"); @@ -2680,11 +2681,11 @@ void tst_QNetworkReply::ioGetFromHttpWithCache_data() "\r\n"; QTest::newRow("not-cached,always-network") - << reply200 << "Reloaded" << MyMemoryCache::CachedContent() << int(QNetworkRequest::AlwaysNetwork) << false << true; + << reply200 << "Reloaded" << MyMemoryCache::CachedContent() << int(QNetworkRequest::AlwaysNetwork) << QStringList() << false << true; QTest::newRow("not-cached,prefer-network") - << reply200 << "Reloaded" << MyMemoryCache::CachedContent() << int(QNetworkRequest::PreferNetwork) << false << true; + << reply200 << "Reloaded" << MyMemoryCache::CachedContent() << int(QNetworkRequest::PreferNetwork) << QStringList() << false << true; QTest::newRow("not-cached,prefer-cache") - << reply200 << "Reloaded" << MyMemoryCache::CachedContent() << int(QNetworkRequest::PreferCache) << false << true; + << reply200 << "Reloaded" << MyMemoryCache::CachedContent() << int(QNetworkRequest::PreferCache) << QStringList() << false << true; QDateTime present = QDateTime::currentDateTime().toUTC(); QDateTime past = present.addSecs(-3600); @@ -2706,14 +2707,14 @@ void tst_QNetworkReply::ioGetFromHttpWithCache_data() content.first.setLastModified(past); QTest::newRow("expired,200,prefer-network") - << reply200 << "Reloaded" << content << int(QNetworkRequest::PreferNetwork) << false << true; + << reply200 << "Reloaded" << content << int(QNetworkRequest::PreferNetwork) << QStringList() << false << true; QTest::newRow("expired,200,prefer-cache") - << reply200 << "Reloaded" << content << int(QNetworkRequest::PreferCache) << false << true; + << reply200 << "Reloaded" << content << int(QNetworkRequest::PreferCache) << QStringList() << false << true; QTest::newRow("expired,304,prefer-network") - << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferNetwork) << true << true; + << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferNetwork) << QStringList() << true << true; QTest::newRow("expired,304,prefer-cache") - << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferCache) << true << true; + << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferCache) << QStringList() << true << true; // // Set to not-expired @@ -2725,20 +2726,20 @@ void tst_QNetworkReply::ioGetFromHttpWithCache_data() content.first.setExpirationDate(future); QTest::newRow("not-expired,200,always-network") - << reply200 << "Reloaded" << content << int(QNetworkRequest::AlwaysNetwork) << false << true; + << reply200 << "Reloaded" << content << int(QNetworkRequest::AlwaysNetwork) << QStringList() << false << true; QTest::newRow("not-expired,200,prefer-network") - << reply200 << "Not-reloaded" << content << int(QNetworkRequest::PreferNetwork) << true << false; + << reply200 << "Not-reloaded" << content << int(QNetworkRequest::PreferNetwork) << QStringList() << true << false; QTest::newRow("not-expired,200,prefer-cache") - << reply200 << "Not-reloaded" << content << int(QNetworkRequest::PreferCache) << true << false; + << reply200 << "Not-reloaded" << content << int(QNetworkRequest::PreferCache) << QStringList() << true << false; QTest::newRow("not-expired,200,always-cache") - << reply200 << "Not-reloaded" << content << int(QNetworkRequest::AlwaysCache) << true << false; + << reply200 << "Not-reloaded" << content << int(QNetworkRequest::AlwaysCache) << QStringList() << true << false; QTest::newRow("not-expired,304,prefer-network") - << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferNetwork) << true << false; + << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferNetwork) << QStringList() << true << false; QTest::newRow("not-expired,304,prefer-cache") - << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferCache) << true << false; + << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferCache) << QStringList() << true << false; QTest::newRow("not-expired,304,always-cache") - << reply304 << "Not-reloaded" << content << int(QNetworkRequest::AlwaysCache) << true << false; + << reply304 << "Not-reloaded" << content << int(QNetworkRequest::AlwaysCache) << QStringList() << true << false; // // Set must-revalidate now @@ -2749,20 +2750,42 @@ void tst_QNetworkReply::ioGetFromHttpWithCache_data() content.first.setRawHeaders(rawHeaders); QTest::newRow("must-revalidate,200,always-network") - << reply200 << "Reloaded" << content << int(QNetworkRequest::AlwaysNetwork) << false << true; + << reply200 << "Reloaded" << content << int(QNetworkRequest::AlwaysNetwork) << QStringList() << false << true; QTest::newRow("must-revalidate,200,prefer-network") - << reply200 << "Reloaded" << content << int(QNetworkRequest::PreferNetwork) << false << true; + << reply200 << "Reloaded" << content << int(QNetworkRequest::PreferNetwork) << QStringList() << false << true; QTest::newRow("must-revalidate,200,prefer-cache") - << reply200 << "Not-reloaded" << content << int(QNetworkRequest::PreferCache) << true << false; + << reply200 << "Not-reloaded" << content << int(QNetworkRequest::PreferCache) << QStringList() << true << false; QTest::newRow("must-revalidate,200,always-cache") - << reply200 << "Not-reloaded" << content << int(QNetworkRequest::AlwaysCache) << true << false; + << reply200 << "Not-reloaded" << content << int(QNetworkRequest::AlwaysCache) << QStringList() << true << false; QTest::newRow("must-revalidate,304,prefer-network") - << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferNetwork) << true << true; + << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferNetwork) << QStringList() << true << true; QTest::newRow("must-revalidate,304,prefer-cache") - << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferCache) << true << false; + << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferCache) << QStringList() << true << false; QTest::newRow("must-revalidate,304,always-cache") - << reply304 << "Not-reloaded" << content << int(QNetworkRequest::AlwaysCache) << true << false; + << reply304 << "Not-reloaded" << content << int(QNetworkRequest::AlwaysCache) << QStringList() << true << false; + + // + // Partial content + // + rawHeaders.clear(); + rawHeaders << QNetworkCacheMetaData::RawHeader("Date", QLocale::c().toString(past, dateFormat).toLatin1()) + << QNetworkCacheMetaData::RawHeader("Cache-control", "max-age=7200"); // isn't used in cache loading + content.first.setRawHeaders(rawHeaders); + content.first.setExpirationDate(future); + + QByteArray reply206 = + "HTTP/1.0 206\r\n" + "Connection: keep-alive\r\n" + "Content-Type: text/plain\r\n" + "Cache-control: no-cache\r\n" + "Content-Range: bytes 2-6/8\r\n" + "Content-length: 4\r\n" + "\r\n" + "load"; + + QTest::newRow("partial,dontuse-cache") + << reply206 << "load" << content << int(QNetworkRequest::PreferCache) << (QStringList() << "Range" << "bytes=2-6") << false << true; } void tst_QNetworkReply::ioGetFromHttpWithCache() @@ -2784,6 +2807,15 @@ void tst_QNetworkReply::ioGetFromHttpWithCache() QNetworkRequest request(url); request.setAttribute(QNetworkRequest::CacheLoadControlAttribute, cacheMode); request.setAttribute(QNetworkRequest::CacheSaveControlAttribute, false); + + QFETCH(QStringList, extraHttpHeaders); + QStringListIterator it(extraHttpHeaders); + while (it.hasNext()) { + QString header = it.next(); + QString value = it.next(); + request.setRawHeader(header.toLatin1(), value.toLatin1()); // To latin1? Deal with it! + } + QNetworkReplyPtr reply = manager.get(request); connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop())); -- cgit v0.12 From 268394b854766855c7ce5697bbc079a6974ad866 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Thu, 17 Mar 2011 15:19:25 +0100 Subject: Fix accidental population of the disk cache with partial content Since the disk cache does not support partial content, we should not try to store it in the cache altogether. Done-with: Jocelyn Turcotte Reviewed-by: Markus Goetz Reviewed-by: Peter Hartmann --- src/network/access/qnetworkreplyimpl.cpp | 7 ++ tests/auto/qnetworkreply/tst_qnetworkreply.cpp | 93 ++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/src/network/access/qnetworkreplyimpl.cpp b/src/network/access/qnetworkreplyimpl.cpp index 343f344..894df79 100644 --- a/src/network/access/qnetworkreplyimpl.cpp +++ b/src/network/access/qnetworkreplyimpl.cpp @@ -505,6 +505,13 @@ void QNetworkReplyImplPrivate::initCacheSaveDevice() { Q_Q(QNetworkReplyImpl); + // The disk cache does not support partial content, so don't even try to + // save any such content into the cache. + if (q->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() == 206) { + cacheEnabled = false; + return; + } + // save the meta data QNetworkCacheMetaData metaData; metaData.setUrl(url); diff --git a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp index 4338e74..81278b6 100644 --- a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp @@ -333,6 +333,8 @@ private Q_SLOTS: void synchronousRequest(); void synchronousRequestSslFailure(); + void dontInsertPartialContentIntoTheCache(); + // NOTE: This test must be last! void parentingRepliesToTheApp(); }; @@ -573,6 +575,63 @@ public: Q_DECLARE_METATYPE(MyMemoryCache::CachedContent) Q_DECLARE_METATYPE(MyMemoryCache::CacheData) +class MySpyMemoryCache: public QAbstractNetworkCache +{ +public: + MySpyMemoryCache(QObject *parent) : QAbstractNetworkCache(parent) {} + ~MySpyMemoryCache() + { + qDeleteAll(m_buffers); + m_buffers.clear(); + } + + QHash m_buffers; + QList m_insertedUrls; + + QNetworkCacheMetaData metaData(const QUrl &) + { + return QNetworkCacheMetaData(); + } + + void updateMetaData(const QNetworkCacheMetaData &) + { + } + + QIODevice *data(const QUrl &) + { + return 0; + } + + bool remove(const QUrl &url) + { + delete m_buffers.take(url); + return m_insertedUrls.removeAll(url) > 0; + } + + qint64 cacheSize() const + { + return 0; + } + + QIODevice *prepare(const QNetworkCacheMetaData &metaData) + { + QBuffer* buffer = new QBuffer; + buffer->open(QIODevice::ReadWrite); + buffer->setProperty("url", metaData.url()); + m_buffers.insert(metaData.url(), buffer); + return buffer; + } + + void insert(QIODevice *buffer) + { + QUrl url = buffer->property("url").toUrl(); + m_insertedUrls << url; + delete m_buffers.take(url); + } + + void clear() { m_insertedUrls.clear(); } +}; + class DataReader: public QObject { Q_OBJECT @@ -5275,6 +5334,39 @@ void tst_QNetworkReply::synchronousRequestSslFailure() QCOMPARE(sslErrorsSpy.count(), 0); } +void tst_QNetworkReply::dontInsertPartialContentIntoTheCache() +{ + QByteArray reply206 = + "HTTP/1.0 206\r\n" + "Connection: keep-alive\r\n" + "Content-Type: text/plain\r\n" + "Cache-control: no-cache\r\n" + "Content-Range: bytes 2-6/8\r\n" + "Content-length: 4\r\n" + "\r\n" + "load"; + + MiniHttpServer server(reply206); + server.doClose = false; + + MySpyMemoryCache *memoryCache = new MySpyMemoryCache(&manager); + manager.setCache(memoryCache); + + QUrl url = "http://localhost:" + QString::number(server.serverPort()); + QNetworkRequest request(url); + request.setRawHeader("Range", "bytes=2-6"); + + QNetworkReplyPtr reply = manager.get(request); + + connect(reply, SIGNAL(finished()), &QTestEventLoop::instance(), SLOT(exitLoop())); + QTestEventLoop::instance().enterLoop(10); + QVERIFY(!QTestEventLoop::instance().timeout()); + + QVERIFY(server.totalConnections > 0); + QCOMPARE(reply->readAll().constData(), "load"); + QCOMPARE(memoryCache->m_insertedUrls.count(), 0); +} + // NOTE: This test must be last testcase in tst_qnetworkreply! void tst_QNetworkReply::parentingRepliesToTheApp() { @@ -5284,4 +5376,5 @@ void tst_QNetworkReply::parentingRepliesToTheApp() } QTEST_MAIN(tst_QNetworkReply) + #include "tst_qnetworkreply.moc" -- cgit v0.12