diff options
author | Mårten Nordheim <marten.nordheim@qt.io> | 2019-03-19 15:49:55 +0100 |
---|---|---|
committer | Mårten Nordheim <marten.nordheim@qt.io> | 2019-03-22 13:25:54 +0000 |
commit | 9e61cec7915ca177e88bd685a3229f153ee7ab7a (patch) | |
tree | 28934f76679f7316133aa11ae5c68a6653be4ec1 | |
parent | 85a9009f258e130a7bd4aaaa252724ea5fb58e79 (diff) |
Network cache: Stop treating no-cache like no-store
In the QNetworkAccessManager machinery we would treat "no-cache" as if
it meant "don't cache" while in reality it means "don't return these
cached elements without making sure they're up-to-date"
At the same time as this change is made let's add test data for
"no-store", which replaces the "no-cache" test data.
Fixes: QTBUG-71896
Change-Id: Ieda98f3982884ccc839cac2420c777968c786f6e
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Reviewed-by: Mikhail Svetkin <mikhail.svetkin@qt.io>
5 files changed, 49 insertions, 24 deletions
diff --git a/src/network/access/qabstractnetworkcache.cpp b/src/network/access/qabstractnetworkcache.cpp index 2b670b2cce..9afb99f23f 100644 --- a/src/network/access/qabstractnetworkcache.cpp +++ b/src/network/access/qabstractnetworkcache.cpp @@ -191,8 +191,8 @@ bool QNetworkCacheMetaData::isValid() const Some cache implementations can keep these cache items in memory for performance reasons, but for security reasons they should not be written to disk. - Specifically with http, documents marked with Pragma: no-cache, or have a Cache-control set to - no-store or no-cache or any https document that doesn't have "Cache-control: public" set will + Specifically with http, documents with Cache-control set to no-store or any + https document that doesn't have "Cache-control: public" set will set the saveToDisk to false. \sa setSaveToDisk() diff --git a/src/network/access/qnetworkaccesscachebackend.cpp b/src/network/access/qnetworkaccesscachebackend.cpp index 0c9a88596d..22fdc5bb0b 100644 --- a/src/network/access/qnetworkaccesscachebackend.cpp +++ b/src/network/access/qnetworkaccesscachebackend.cpp @@ -87,15 +87,16 @@ bool QNetworkAccessCacheBackend::sendCacheContents() setAttribute(QNetworkRequest::HttpReasonPhraseAttribute, attributes.value(QNetworkRequest::HttpReasonPhraseAttribute)); // set the raw headers - QNetworkCacheMetaData::RawHeaderList rawHeaders = item.rawHeaders(); - QNetworkCacheMetaData::RawHeaderList::ConstIterator it = rawHeaders.constBegin(), - end = rawHeaders.constEnd(); - for ( ; it != end; ++it) { - if (it->first.toLower() == "cache-control" && - it->second.toLower().contains("must-revalidate")) { - return false; + const QNetworkCacheMetaData::RawHeaderList rawHeaders = item.rawHeaders(); + for (const auto &header : rawHeaders) { + if (header.first.toLower() == "cache-control") { + const QByteArray cacheControlValue = header.second.toLower(); + if (cacheControlValue.contains("must-revalidate") + || cacheControlValue.contains("no-cache")) { + return false; + } } - setRawHeader(it->first, it->second); + setRawHeader(header.first, header.second); } // handle a possible redirect diff --git a/src/network/access/qnetworkreplyhttpimpl.cpp b/src/network/access/qnetworkreplyhttpimpl.cpp index d4d3a21a7a..2d7649fa61 100644 --- a/src/network/access/qnetworkreplyhttpimpl.cpp +++ b/src/network/access/qnetworkreplyhttpimpl.cpp @@ -524,6 +524,8 @@ bool QNetworkReplyHttpImplPrivate::loadFromCacheIfAllowed(QHttpNetworkRequest &h QHash<QByteArray, QByteArray> cacheControl = parseHttpOptionHeader(it->second); if (cacheControl.contains("must-revalidate")) return false; + if (cacheControl.contains("no-cache")) + return false; } QDateTime currentDateTime = QDateTime::currentDateTimeUtc(); @@ -1730,18 +1732,8 @@ QNetworkCacheMetaData QNetworkReplyHttpImplPrivate::fetchCacheMetaData(const QNe if (httpRequest.operation() == QHttpNetworkRequest::Get) { canDiskCache = true; - // 14.32 - // HTTP/1.1 caches SHOULD treat "Pragma: no-cache" as if the client - // had sent "Cache-Control: no-cache". - it = cacheHeaders.findRawHeader("pragma"); - if (it != cacheHeaders.rawHeaders.constEnd() - && it->second == "no-cache") - canDiskCache = false; - // HTTP/1.1. Check the Cache-Control header - if (cacheControl.contains("no-cache")) - canDiskCache = false; - else if (cacheControl.contains("no-store")) + if (cacheControl.contains("no-store")) canDiskCache = false; // responses to POST might be cacheable diff --git a/tests/auto/network/access/qabstractnetworkcache/tst_qabstractnetworkcache.cpp b/tests/auto/network/access/qabstractnetworkcache/tst_qabstractnetworkcache.cpp index 0da42b8b87..b8d9adf7a1 100644 --- a/tests/auto/network/access/qabstractnetworkcache/tst_qabstractnetworkcache.cpp +++ b/tests/auto/network/access/qabstractnetworkcache/tst_qabstractnetworkcache.cpp @@ -251,9 +251,14 @@ void tst_QAbstractNetworkCache::cacheControl_data() QTest::newRow("200-1") << QNetworkRequest::PreferNetwork << "httpcachetest_cachecontrol-expire.cgi" << false; QTest::newRow("200-2") << QNetworkRequest::AlwaysNetwork << "httpcachetest_cachecontrol.cgi?no-cache" << AlwaysFalse; - QTest::newRow("200-3") << QNetworkRequest::PreferNetwork << "httpcachetest_cachecontrol.cgi?no-cache" << false; + QTest::newRow("200-3") << QNetworkRequest::PreferNetwork << "httpcachetest_cachecontrol.cgi?no-cache" << true; QTest::newRow("200-4") << QNetworkRequest::AlwaysCache << "httpcachetest_cachecontrol.cgi?no-cache" << false; - QTest::newRow("200-5") << QNetworkRequest::PreferCache << "httpcachetest_cachecontrol.cgi?no-cache" << false; + QTest::newRow("200-5") << QNetworkRequest::PreferCache << "httpcachetest_cachecontrol.cgi?no-cache" << true; + + QTest::newRow("200-6") << QNetworkRequest::AlwaysNetwork << "httpcachetest_cachecontrol.cgi?no-store" << AlwaysFalse; + QTest::newRow("200-7") << QNetworkRequest::PreferNetwork << "httpcachetest_cachecontrol.cgi?no-store" << false; + QTest::newRow("200-8") << QNetworkRequest::AlwaysCache << "httpcachetest_cachecontrol.cgi?no-store" << false; + QTest::newRow("200-9") << QNetworkRequest::PreferCache << "httpcachetest_cachecontrol.cgi?no-store" << false; QTest::newRow("304-0") << QNetworkRequest::PreferNetwork << "httpcachetest_cachecontrol.cgi?max-age=1000" << true; diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp index 30b41da515..3876621983 100644 --- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp @@ -3959,7 +3959,7 @@ void tst_QNetworkReply::ioGetFromHttpWithCache_data() "HTTP/1.0 200\r\n" "Connection: keep-alive\r\n" "Content-Type: text/plain\r\n" - "Cache-control: no-cache\r\n" + "Cache-control: no-store\r\n" "Content-length: 8\r\n" "\r\n" "Reloaded"; @@ -3985,6 +3985,33 @@ void tst_QNetworkReply::ioGetFromHttpWithCache_data() content.second = "Not-reloaded"; content.first.setLastModified(past); + // "no-cache" + rawHeaders.clear(); + rawHeaders << QNetworkCacheMetaData::RawHeader("Date", QLocale::c().toString(past, dateFormat).toLatin1()) + << QNetworkCacheMetaData::RawHeader("Cache-control", "no-cache"); + content.first.setRawHeaders(rawHeaders); + content.first.setLastModified(past); + content.first.setExpirationDate(future); + + // "no-cache" does not mean "no cache", just that we must consult remote first + QTest::newRow("no-cache,200,always-network") + << reply200 << "Reloaded" << content << int(QNetworkRequest::AlwaysNetwork) << QStringList() << false << true; + QTest::newRow("no-cache,200,prefer-network") + << reply200 << "Reloaded" << content << int(QNetworkRequest::PreferNetwork) << QStringList() << false << true; + QTest::newRow("no-cache,200,prefer-cache") + << reply200 << "Reloaded" << content << int(QNetworkRequest::PreferCache) << QStringList() << false << true; + // We're not allowed by the spec to deliver cached data without checking if it is still + // up-to-date. + QTest::newRow("no-cache,200,always-cache") + << reply200 << QString() << content << int(QNetworkRequest::AlwaysCache) << QStringList() << false << false; + + QTest::newRow("no-cache,304,prefer-network") + << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferNetwork) << QStringList() << true << true; + QTest::newRow("no-cache,304,prefer-cache") + << reply304 << "Not-reloaded" << content << int(QNetworkRequest::PreferCache) << QStringList() << true << true; + QTest::newRow("no-cache,304,always-cache") + << reply304 << QString() << content << int(QNetworkRequest::AlwaysCache) << QStringList() << false << false; + // // Set to expired // |