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>

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()

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

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

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;
 

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
     //