Respect cache-control directives in request
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Mar 2015 15:13:02 +0000 (15:13 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Mar 2015 15:13:02 +0000 (15:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=143121
Source/WebCore:

rdar://problem/19714040

Reviewed by Chris Dumez.

Test: http/tests/cache/disk-cache/disk-cache-request-headers.html

* loader/cache/CacheValidation.cpp:
(WebCore::isCacheHeaderSeparator):
(WebCore::isControlCharacter):
(WebCore::trimToNextSeparator):
(WebCore::parseCacheHeader):
(WebCore::parseCacheControlDirectives):

    Factor Cache-control parsing here so it can be used for both requests and responses.

* loader/cache/CacheValidation.h:
* platform/network/ResourceRequestBase.h:
* platform/network/ResourceResponseBase.cpp:
(WebCore::ResourceResponseBase::ResourceResponseBase):
(WebCore::ResourceResponseBase::parseCacheControlDirectives):
(WebCore::ResourceResponseBase::cacheControlContainsNoCache):
(WebCore::ResourceResponseBase::cacheControlContainsNoStore):
(WebCore::ResourceResponseBase::cacheControlContainsMustRevalidate):
(WebCore::ResourceResponseBase::cacheControlMaxAge):
(WebCore::isCacheHeaderSeparator): Deleted.
(WebCore::isControlCharacter): Deleted.
(WebCore::trimToNextSeparator): Deleted.
(WebCore::parseCacheHeader): Deleted.
* platform/network/ResourceResponseBase.h:

Source/WebKit2:

rdar://problem/19714040

Reviewed by Chris Dumez.

Better support for https://tools.ietf.org/html/rfc7234#section-5.2.1

* NetworkProcess/cache/NetworkCache.cpp:
(WebKit::NetworkCache::canUse):

    Consider requests with Cache-control: no-cache and max-age=0 expired.

(WebKit::NetworkCache::canStore):

    Don't store requests with Cache-control: no-store.

(WebKit::NetworkCache::Cache::store):
* NetworkProcess/cache/NetworkCache.h:
* NetworkProcess/cache/NetworkCacheStatisticsCocoa.mm:
(WebKit::NetworkCache::storeDecisionToDiagnosticKey):

LayoutTests:

Reviewed by Chris Dumez.

* http/tests/cache/disk-cache/disk-cache-request-headers-expected.txt: Added.
* http/tests/cache/disk-cache/disk-cache-request-headers.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@182059 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/cache/disk-cache/disk-cache-request-headers-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/cache/disk-cache/disk-cache-request-headers.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CacheValidation.cpp
Source/WebCore/loader/cache/CacheValidation.h
Source/WebCore/platform/network/ResourceRequestBase.h
Source/WebCore/platform/network/ResourceResponseBase.cpp
Source/WebCore/platform/network/ResourceResponseBase.h
Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCache.h
Source/WebKit2/NetworkProcess/cache/NetworkCacheStatisticsCocoa.mm

index 96fb172..10b1d04 100644 (file)
@@ -1,3 +1,13 @@
+2015-03-26  Antti Koivisto  <antti@apple.com>
+
+        Respect cache-control directives in request
+        https://bugs.webkit.org/show_bug.cgi?id=143121
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/cache/disk-cache/disk-cache-request-headers-expected.txt: Added.
+        * http/tests/cache/disk-cache/disk-cache-request-headers.html: Added.
+
 2015-03-27  Michael Saboff  <msaboff@apple.com>
 
         Objects with numeric properties intermittently get a phantom 'length' property
diff --git a/LayoutTests/http/tests/cache/disk-cache/disk-cache-request-headers-expected.txt b/LayoutTests/http/tests/cache/disk-cache/disk-cache-request-headers-expected.txt
new file mode 100644 (file)
index 0000000..e72d355
--- /dev/null
@@ -0,0 +1,105 @@
+Test permutations of various cache headers
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+running 24 tests
+
+response headers: {"Cache-control":"max-age=0"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+response source: Disk cache
+
+response headers: {"Cache-control":"max-age=0"}
+request headers: {"Cache-control":"no-cache"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+request headers: {"Cache-control":"no-cache"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=0"}
+request headers: {"Cache-control":"no-store"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+request headers: {"Cache-control":"no-store"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=0"}
+request headers: {"Cache-control":"no-cache, no-store"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+request headers: {"Cache-control":"no-cache, no-store"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=0"}
+request headers: {"Cache-control":"max-age=0"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+request headers: {"Cache-control":"max-age=0"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=0"}
+request headers: {"Cache-control":"no-cache, max-age=0"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+request headers: {"Cache-control":"no-cache, max-age=0"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=0"}
+request headers: {"Cache-control":"no-store, max-age=0"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+request headers: {"Cache-control":"no-store, max-age=0"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=0"}
+request headers: {"Cache-control":"no-cache, no-store, max-age=0"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+request headers: {"Cache-control":"no-cache, no-store, max-age=0"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=0"}
+request headers: {"Cache-control":"max-age=100"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+request headers: {"Cache-control":"max-age=100"}
+response source: Disk cache
+
+response headers: {"Cache-control":"max-age=0"}
+request headers: {"Cache-control":"no-cache, max-age=100"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+request headers: {"Cache-control":"no-cache, max-age=100"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=0"}
+request headers: {"Cache-control":"no-store, max-age=100"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+request headers: {"Cache-control":"no-store, max-age=100"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=0"}
+request headers: {"Cache-control":"no-cache, no-store, max-age=100"}
+response source: Network
+
+response headers: {"Cache-control":"max-age=100"}
+request headers: {"Cache-control":"no-cache, no-store, max-age=100"}
+response source: Network
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/cache/disk-cache/disk-cache-request-headers.html b/LayoutTests/http/tests/cache/disk-cache/disk-cache-request-headers.html
new file mode 100644 (file)
index 0000000..5f640c9
--- /dev/null
@@ -0,0 +1,37 @@
+<script src="/js-test-resources/js-test-pre.js"></script>
+<script src="resources/cache-test.js"></script>
+<body>
+<script>
+
+var testMatrix =
+[
+ [
+  { responseHeaders: {'Cache-control': 'max-age=0' } },
+  { responseHeaders: {'Cache-control': 'max-age=100' } },
+  ],
+ [
+  {},
+  { requestHeaders: {'Cache-control': 'no-cache' } },
+  ],
+ [
+  {},
+  { requestHeaders: {'Cache-control': 'no-store' } },
+  ],
+ [
+  {},
+  { requestHeaders: {'Cache-control': 'max-age=0' } },
+  { requestHeaders: {'Cache-control': 'max-age=100' } },
+  ],
+ ];
+
+description("Test permutations of various cache headers");
+
+var tests = generateTests(testMatrix);
+
+debug("running " + tests.length + " tests");
+debug("");
+
+runTests(tests);
+
+</script>
+<script src="/js-test-resources/js-test-post.js"></script>
index 95235f7..8a91736 100644 (file)
@@ -1,3 +1,37 @@
+2015-03-26  Antti Koivisto  <antti@apple.com>
+
+        Respect cache-control directives in request
+        https://bugs.webkit.org/show_bug.cgi?id=143121
+        rdar://problem/19714040
+
+        Reviewed by Chris Dumez.
+
+        Test: http/tests/cache/disk-cache/disk-cache-request-headers.html
+
+        * loader/cache/CacheValidation.cpp:
+        (WebCore::isCacheHeaderSeparator):
+        (WebCore::isControlCharacter):
+        (WebCore::trimToNextSeparator):
+        (WebCore::parseCacheHeader):
+        (WebCore::parseCacheControlDirectives):
+
+            Factor Cache-control parsing here so it can be used for both requests and responses.
+
+        * loader/cache/CacheValidation.h:
+        * platform/network/ResourceRequestBase.h:
+        * platform/network/ResourceResponseBase.cpp:
+        (WebCore::ResourceResponseBase::ResourceResponseBase):
+        (WebCore::ResourceResponseBase::parseCacheControlDirectives):
+        (WebCore::ResourceResponseBase::cacheControlContainsNoCache):
+        (WebCore::ResourceResponseBase::cacheControlContainsNoStore):
+        (WebCore::ResourceResponseBase::cacheControlContainsMustRevalidate):
+        (WebCore::ResourceResponseBase::cacheControlMaxAge):
+        (WebCore::isCacheHeaderSeparator): Deleted.
+        (WebCore::isControlCharacter): Deleted.
+        (WebCore::trimToNextSeparator): Deleted.
+        (WebCore::parseCacheHeader): Deleted.
+        * platform/network/ResourceResponseBase.h:
+
 2015-03-27  Víctor Manuel Jáquez Leal  <vjaquez@igalia.com>
 
         [GStreamer] share GL context in pipeline, part 2
index 7a013ec..b2e361d 100644 (file)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "CacheValidation.h"
 
+#include "HTTPHeaderMap.h"
 #include "ResourceResponse.h"
 #include <wtf/CurrentTime.h>
 
@@ -149,4 +150,144 @@ bool redirectChainAllowsReuse(RedirectChainCacheStatus redirectChainCacheStatus,
     return false;
 }
 
+inline bool isCacheHeaderSeparator(UChar c)
+{
+    // See RFC 2616, Section 2.2
+    switch (c) {
+    case '(':
+    case ')':
+    case '<':
+    case '>':
+    case '@':
+    case ',':
+    case ';':
+    case ':':
+    case '\\':
+    case '"':
+    case '/':
+    case '[':
+    case ']':
+    case '?':
+    case '=':
+    case '{':
+    case '}':
+    case ' ':
+    case '\t':
+        return true;
+    default:
+        return false;
+    }
+}
+
+inline bool isControlCharacter(UChar c)
+{
+    return c < ' ' || c == 127;
+}
+
+inline String trimToNextSeparator(const String& str)
+{
+    return str.substring(0, str.find(isCacheHeaderSeparator));
+}
+
+static Vector<std::pair<String, String>> parseCacheHeader(const String& header)
+{
+    Vector<std::pair<String, String>> result;
+
+    const String safeHeader = header.removeCharacters(isControlCharacter);
+    unsigned max = safeHeader.length();
+    unsigned pos = 0;
+    while (pos < max) {
+        size_t nextCommaPosition = safeHeader.find(',', pos);
+        size_t nextEqualSignPosition = safeHeader.find('=', pos);
+        if (nextEqualSignPosition == notFound && nextCommaPosition == notFound) {
+            // Add last directive to map with empty string as value
+            result.append(std::make_pair(trimToNextSeparator(safeHeader.substring(pos, max - pos).stripWhiteSpace()), ""));
+            return result;
+        }
+        if (nextCommaPosition != notFound && (nextCommaPosition < nextEqualSignPosition || nextEqualSignPosition == notFound)) {
+            // Add directive to map with empty string as value
+            result.append(std::make_pair(trimToNextSeparator(safeHeader.substring(pos, nextCommaPosition - pos).stripWhiteSpace()), ""));
+            pos += nextCommaPosition - pos + 1;
+            continue;
+        }
+        // Get directive name, parse right hand side of equal sign, then add to map
+        String directive = trimToNextSeparator(safeHeader.substring(pos, nextEqualSignPosition - pos).stripWhiteSpace());
+        pos += nextEqualSignPosition - pos + 1;
+
+        String value = safeHeader.substring(pos, max - pos).stripWhiteSpace();
+        if (value[0] == '"') {
+            // The value is a quoted string
+            size_t nextDoubleQuotePosition = value.find('"', 1);
+            if (nextDoubleQuotePosition == notFound) {
+                // Parse error; just use the rest as the value
+                result.append(std::make_pair(directive, trimToNextSeparator(value.substring(1, value.length() - 1).stripWhiteSpace())));
+                return result;
+            }
+            // Store the value as a quoted string without quotes
+            result.append(std::make_pair(directive, value.substring(1, nextDoubleQuotePosition - 1).stripWhiteSpace()));
+            pos += (safeHeader.find('"', pos) - pos) + nextDoubleQuotePosition + 1;
+            // Move past next comma, if there is one
+            size_t nextCommaPosition2 = safeHeader.find(',', pos);
+            if (nextCommaPosition2 == notFound)
+                return result; // Parse error if there is anything left with no comma
+            pos += nextCommaPosition2 - pos + 1;
+            continue;
+        }
+        // The value is a token until the next comma
+        size_t nextCommaPosition2 = value.find(',');
+        if (nextCommaPosition2 == notFound) {
+            // The rest is the value; no change to value needed
+            result.append(std::make_pair(directive, trimToNextSeparator(value)));
+            return result;
+        }
+        // The value is delimited by the next comma
+        result.append(std::make_pair(directive, trimToNextSeparator(value.substring(0, nextCommaPosition2).stripWhiteSpace())));
+        pos += (safeHeader.find(',', pos) - pos) + 1;
+    }
+    return result;
+}
+
+CacheControlDirectives parseCacheControlDirectives(const HTTPHeaderMap& headers)
+{
+    CacheControlDirectives result;
+
+    String cacheControlValue = headers.get(HTTPHeaderName::CacheControl);
+    if (!cacheControlValue.isEmpty()) {
+        auto directives = parseCacheHeader(cacheControlValue);
+
+        size_t directivesSize = directives.size();
+        for (size_t i = 0; i < directivesSize; ++i) {
+            // RFC2616 14.9.1: A no-cache directive with a value is only meaningful for proxy caches.
+            // It should be ignored by a browser level cache.
+            if (equalIgnoringCase(directives[i].first, "no-cache") && directives[i].second.isEmpty())
+                result.noCache = true;
+            else if (equalIgnoringCase(directives[i].first, "no-store"))
+                result.noStore = true;
+            else if (equalIgnoringCase(directives[i].first, "must-revalidate"))
+                result.mustRevalidate = true;
+            else if (equalIgnoringCase(directives[i].first, "max-age")) {
+                if (!std::isnan(result.maxAge)) {
+                    // First max-age directive wins if there are multiple ones.
+                    continue;
+                }
+                bool ok;
+                double maxAge = directives[i].second.toDouble(&ok);
+                if (ok)
+                    result.maxAge = maxAge;
+            }
+        }
+    }
+
+    if (!result.noCache) {
+        // Handle Pragma: no-cache
+        // This is deprecated and equivalent to Cache-control: no-cache
+        // Don't bother tokenizing the value, it is not important
+        String pragmaValue = headers.get(HTTPHeaderName::Pragma);
+
+        result.noCache = pragmaValue.contains("no-cache", false);
+    }
+
+    return result;
+}
+
 }
index 0f1affd..b828bcf 100644 (file)
@@ -28,6 +28,7 @@
 
 namespace WebCore {
 
+class HTTPHeaderMap;
 class ResourceResponse;
 
 struct RedirectChainCacheStatus {
@@ -52,6 +53,14 @@ WEBCORE_EXPORT void updateRedirectChainStatus(RedirectChainCacheStatus&, const R
 enum ReuseExpiredRedirectionOrNot { DoNotReuseExpiredRedirection, ReuseExpiredRedirection };
 WEBCORE_EXPORT bool redirectChainAllowsReuse(RedirectChainCacheStatus, ReuseExpiredRedirectionOrNot);
 
+struct CacheControlDirectives {
+    double maxAge { std::numeric_limits<double>::quiet_NaN() };
+    bool noCache { false };
+    bool noStore { false };
+    bool mustRevalidate { false };
+};
+WEBCORE_EXPORT CacheControlDirectives parseCacheControlDirectives(const HTTPHeaderMap&);
+
 }
 
 #endif
index ecae227..c69aaf9 100644 (file)
@@ -79,7 +79,7 @@ namespace WebCore {
         WEBCORE_EXPORT const String& httpMethod() const;
         WEBCORE_EXPORT void setHTTPMethod(const String& httpMethod);
         
-        const HTTPHeaderMap& httpHeaderFields() const;
+        WEBCORE_EXPORT const HTTPHeaderMap& httpHeaderFields() const;
         WEBCORE_EXPORT void setHTTPHeaderFields(HTTPHeaderMap);
 
         WEBCORE_EXPORT String httpHeaderField(const String& name) const;
index 1b18b8a..db80415 100644 (file)
@@ -27,6 +27,7 @@
 #include "config.h"
 #include "ResourceResponseBase.h"
 
+#include "CacheValidation.h"
 #include "HTTPHeaderNames.h"
 #include "HTTPParsers.h"
 #include "ResourceResponse.h"
@@ -37,8 +38,6 @@
 
 namespace WebCore {
 
-static void parseCacheHeader(const String& header, Vector<std::pair<String, String>>& result);
-
 inline const ResourceResponse& ResourceResponseBase::asResourceResponse() const
 {
     return *static_cast<const ResourceResponse*>(this);
@@ -48,7 +47,6 @@ ResourceResponseBase::ResourceResponseBase()
     : m_expectedContentLength(0)
     , m_includesCertificateInfo(false)
     , m_httpStatusCode(0)
-    , m_cacheControlMaxAge(0)
     , m_age(0)
     , m_date(0)
     , m_expires(0)
@@ -59,9 +57,6 @@ ResourceResponseBase::ResourceResponseBase()
     , m_haveParsedDateHeader(false)
     , m_haveParsedExpiresHeader(false)
     , m_haveParsedLastModifiedHeader(false)
-    , m_cacheControlContainsNoCache(false)
-    , m_cacheControlContainsNoStore(false)
-    , m_cacheControlContainsMustRevalidate(false)
     , m_source(Source::Unknown)
 {
 }
@@ -73,7 +68,6 @@ ResourceResponseBase::ResourceResponseBase(const URL& url, const String& mimeTyp
     , m_textEncodingName(textEncodingName)
     , m_includesCertificateInfo(true) // Empty but valid for synthetic responses.
     , m_httpStatusCode(0)
-    , m_cacheControlMaxAge(0)
     , m_age(0)
     , m_date(0)
     , m_expires(0)
@@ -84,9 +78,6 @@ ResourceResponseBase::ResourceResponseBase(const URL& url, const String& mimeTyp
     , m_haveParsedDateHeader(false)
     , m_haveParsedExpiresHeader(false)
     , m_haveParsedLastModifiedHeader(false)
-    , m_cacheControlContainsNoCache(false)
-    , m_cacheControlContainsNoStore(false)
-    , m_cacheControlContainsMustRevalidate(false)
     , m_source(Source::Unknown)
 {
 }
@@ -358,69 +349,29 @@ void ResourceResponseBase::parseCacheControlDirectives() const
 
     lazyInit(CommonFieldsOnly);
 
+    m_cacheControlDirectives = WebCore::parseCacheControlDirectives(m_httpHeaderFields);
     m_haveParsedCacheControlHeader = true;
-
-    m_cacheControlContainsMustRevalidate = false;
-    m_cacheControlContainsNoCache = false;
-    m_cacheControlMaxAge = std::numeric_limits<double>::quiet_NaN();
-
-    String cacheControlValue = m_httpHeaderFields.get(HTTPHeaderName::CacheControl);
-    if (!cacheControlValue.isEmpty()) {
-        Vector<std::pair<String, String>> directives;
-        parseCacheHeader(cacheControlValue, directives);
-
-        size_t directivesSize = directives.size();
-        for (size_t i = 0; i < directivesSize; ++i) {
-            // RFC2616 14.9.1: A no-cache directive with a value is only meaningful for proxy caches.
-            // It should be ignored by a browser level cache.
-            if (equalIgnoringCase(directives[i].first, "no-cache") && directives[i].second.isEmpty())
-                m_cacheControlContainsNoCache = true;
-            else if (equalIgnoringCase(directives[i].first, "no-store"))
-                m_cacheControlContainsNoStore = true;
-            else if (equalIgnoringCase(directives[i].first, "must-revalidate"))
-                m_cacheControlContainsMustRevalidate = true;
-            else if (equalIgnoringCase(directives[i].first, "max-age")) {
-                if (!std::isnan(m_cacheControlMaxAge)) {
-                    // First max-age directive wins if there are multiple ones.
-                    continue;
-                }
-                bool ok;
-                double maxAge = directives[i].second.toDouble(&ok);
-                if (ok)
-                    m_cacheControlMaxAge = maxAge;
-            }
-        }
-    }
-
-    if (!m_cacheControlContainsNoCache) {
-        // Handle Pragma: no-cache
-        // This is deprecated and equivalent to Cache-control: no-cache
-        // Don't bother tokenizing the value, it is not important
-        String pragmaValue = m_httpHeaderFields.get(HTTPHeaderName::Pragma);
-
-        m_cacheControlContainsNoCache = pragmaValue.contains("no-cache", false);
-    }
 }
     
 bool ResourceResponseBase::cacheControlContainsNoCache() const
 {
     if (!m_haveParsedCacheControlHeader)
         parseCacheControlDirectives();
-    return m_cacheControlContainsNoCache;
+    return m_cacheControlDirectives.noCache;
 }
 
 bool ResourceResponseBase::cacheControlContainsNoStore() const
 {
     if (!m_haveParsedCacheControlHeader)
         parseCacheControlDirectives();
-    return m_cacheControlContainsNoStore;
+    return m_cacheControlDirectives.noStore;
 }
 
 bool ResourceResponseBase::cacheControlContainsMustRevalidate() const
 {
     if (!m_haveParsedCacheControlHeader)
         parseCacheControlDirectives();
-    return m_cacheControlContainsMustRevalidate;
+    return m_cacheControlDirectives.mustRevalidate;
 }
 
 bool ResourceResponseBase::hasCacheValidatorFields() const
@@ -434,7 +385,7 @@ double ResourceResponseBase::cacheControlMaxAge() const
 {
     if (!m_haveParsedCacheControlHeader)
         parseCacheControlDirectives();
-    return m_cacheControlMaxAge;
+    return m_cacheControlDirectives.maxAge;
 }
 
 static double parseDateValueInHeader(const HTTPHeaderMap& headers, HTTPHeaderName headerName)
@@ -555,99 +506,4 @@ bool ResourceResponseBase::compare(const ResourceResponse& a, const ResourceResp
     return ResourceResponse::platformCompare(a, b);
 }
 
-static bool isCacheHeaderSeparator(UChar c)
-{
-    // See RFC 2616, Section 2.2
-    switch (c) {
-        case '(':
-        case ')':
-        case '<':
-        case '>':
-        case '@':
-        case ',':
-        case ';':
-        case ':':
-        case '\\':
-        case '"':
-        case '/':
-        case '[':
-        case ']':
-        case '?':
-        case '=':
-        case '{':
-        case '}':
-        case ' ':
-        case '\t':
-            return true;
-        default:
-            return false;
-    }
-}
-
-static bool isControlCharacter(UChar c)
-{
-    return c < ' ' || c == 127;
-}
-
-static inline String trimToNextSeparator(const String& str)
-{
-    return str.substring(0, str.find(isCacheHeaderSeparator));
-}
-
-static void parseCacheHeader(const String& header, Vector<std::pair<String, String>>& result)
-{
-    const String safeHeader = header.removeCharacters(isControlCharacter);
-    unsigned max = safeHeader.length();
-    for (unsigned pos = 0; pos < max; /* pos incremented in loop */) {
-        size_t nextCommaPosition = safeHeader.find(',', pos);
-        size_t nextEqualSignPosition = safeHeader.find('=', pos);
-        if (nextEqualSignPosition != notFound && (nextEqualSignPosition < nextCommaPosition || nextCommaPosition == notFound)) {
-            // Get directive name, parse right hand side of equal sign, then add to map
-            String directive = trimToNextSeparator(safeHeader.substring(pos, nextEqualSignPosition - pos).stripWhiteSpace());
-            pos += nextEqualSignPosition - pos + 1;
-
-            String value = safeHeader.substring(pos, max - pos).stripWhiteSpace();
-            if (value[0] == '"') {
-                // The value is a quoted string
-                size_t nextDoubleQuotePosition = value.find('"', 1);
-                if (nextDoubleQuotePosition != notFound) {
-                    // Store the value as a quoted string without quotes
-                    result.append(std::pair<String, String>(directive, value.substring(1, nextDoubleQuotePosition - 1).stripWhiteSpace()));
-                    pos += (safeHeader.find('"', pos) - pos) + nextDoubleQuotePosition + 1;
-                    // Move past next comma, if there is one
-                    size_t nextCommaPosition2 = safeHeader.find(',', pos);
-                    if (nextCommaPosition2 != notFound)
-                        pos += nextCommaPosition2 - pos + 1;
-                    else
-                        return; // Parse error if there is anything left with no comma
-                } else {
-                    // Parse error; just use the rest as the value
-                    result.append(std::pair<String, String>(directive, trimToNextSeparator(value.substring(1, value.length() - 1).stripWhiteSpace())));
-                    return;
-                }
-            } else {
-                // The value is a token until the next comma
-                size_t nextCommaPosition2 = value.find(',');
-                if (nextCommaPosition2 != notFound) {
-                    // The value is delimited by the next comma
-                    result.append(std::pair<String, String>(directive, trimToNextSeparator(value.substring(0, nextCommaPosition2).stripWhiteSpace())));
-                    pos += (safeHeader.find(',', pos) - pos) + 1;
-                } else {
-                    // The rest is the value; no change to value needed
-                    result.append(std::pair<String, String>(directive, trimToNextSeparator(value)));
-                    return;
-                }
-            }
-        } else if (nextCommaPosition != notFound && (nextCommaPosition < nextEqualSignPosition || nextEqualSignPosition == notFound)) {
-            // Add directive to map with empty string as value
-            result.append(std::pair<String, String>(trimToNextSeparator(safeHeader.substring(pos, nextCommaPosition - pos).stripWhiteSpace()), ""));
-            pos += nextCommaPosition - pos + 1;
-        } else {
-            // Add last directive to map with empty string as value
-            result.append(std::pair<String, String>(trimToNextSeparator(safeHeader.substring(pos, max - pos).stripWhiteSpace()), ""));
-            return;
-        }
-    }
-}
-
 }
index 2350a4a..d73656c 100644 (file)
@@ -27,6 +27,7 @@
 #ifndef ResourceResponseBase_h
 #define ResourceResponseBase_h
 
+#include "CacheValidation.h"
 #include "CertificateInfo.h"
 #include "HTTPHeaderMap.h"
 #include "URL.h"
@@ -160,11 +161,11 @@ protected:
     int m_httpStatusCode;
 
 private:
-    mutable double m_cacheControlMaxAge;
     mutable double m_age;
     mutable double m_date;
     mutable double m_expires;
     mutable double m_lastModified;
+    mutable CacheControlDirectives m_cacheControlDirectives;
 
 public:
     bool m_isNull : 1;
@@ -180,10 +181,6 @@ private:
     mutable bool m_haveParsedExpiresHeader : 1;
     mutable bool m_haveParsedLastModifiedHeader : 1;
 
-    mutable bool m_cacheControlContainsNoCache : 1;
-    mutable bool m_cacheControlContainsNoStore : 1;
-    mutable bool m_cacheControlContainsMustRevalidate : 1;
-
     Source m_source;
 };
 
index bcd6825..ff0c778 100644 (file)
@@ -1,3 +1,27 @@
+2015-03-26  Antti Koivisto  <antti@apple.com>
+
+        Respect cache-control directives in request
+        https://bugs.webkit.org/show_bug.cgi?id=143121
+        rdar://problem/19714040
+
+        Reviewed by Chris Dumez.
+
+        Better support for https://tools.ietf.org/html/rfc7234#section-5.2.1
+
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::canUse):
+
+            Consider requests with Cache-control: no-cache and max-age=0 expired.
+
+        (WebKit::NetworkCache::canStore):
+
+            Don't store requests with Cache-control: no-store.
+
+        (WebKit::NetworkCache::Cache::store):
+        * NetworkProcess/cache/NetworkCache.h:
+        * NetworkProcess/cache/NetworkCacheStatisticsCocoa.mm:
+        (WebKit::NetworkCache::storeDecisionToDiagnosticKey):
+
 2015-03-25  Jon Honeycutt  <jhoneycutt@apple.com>
 
         iOS file upload panel menu items need icons
index ca8aa7a..e461d4f 100644 (file)
@@ -150,7 +150,7 @@ static bool cachePolicyAllowsExpired(WebCore::ResourceRequestCachePolicy policy)
     return false;
 }
 
-static inline bool responseHasExpired(const WebCore::ResourceResponse& response, std::chrono::milliseconds timestamp)
+static bool responseHasExpired(const WebCore::ResourceResponse& response, std::chrono::milliseconds timestamp)
 {
     if (response.cacheControlContainsNoCache())
         return true;
@@ -169,6 +169,18 @@ static inline bool responseHasExpired(const WebCore::ResourceResponse& response,
     return hasExpired;
 }
 
+static bool requestNeedsRevalidation(const WebCore::ResourceRequest& request)
+{
+    auto requestDirectives = WebCore::parseCacheControlDirectives(request.httpHeaderFields());
+    if (requestDirectives.noCache)
+        return true;
+    // For requests we ignore max-age values other than zero.
+    if (requestDirectives.maxAge == 0)
+        return true;
+
+    return false;
+}
+
 static UseDecision canUse(const Entry& entry, const WebCore::ResourceRequest& request)
 {
     if (!verifyVaryingRequestHeaders(entry.varyingRequestHeaders(), request)) {
@@ -176,8 +188,11 @@ static UseDecision canUse(const Entry& entry, const WebCore::ResourceRequest& re
         return UseDecision::NoDueToVaryingHeaderMismatch;
     }
 
-    // We never revalidate in the case of a history navigation (i.e. cachePolicyAllowsExpired() returns true).
-    bool needsRevalidation = !cachePolicyAllowsExpired(request.cachePolicy()) && responseHasExpired(entry.response(), entry.timeStamp());
+    // We never revalidate in the case of a history navigation.
+    if (cachePolicyAllowsExpired(request.cachePolicy()))
+        return UseDecision::Use;
+
+    bool needsRevalidation = requestNeedsRevalidation(request) || responseHasExpired(entry.response(), entry.timeStamp());
     if (!needsRevalidation)
         return UseDecision::Use;
 
@@ -266,14 +281,18 @@ void Cache::retrieve(const WebCore::ResourceRequest& originalRequest, uint64_t w
 
 static StoreDecision canStore(const WebCore::ResourceRequest& originalRequest, const WebCore::ResourceResponse& response)
 {
-    if (!originalRequest.url().protocolIsInHTTPFamily() || !response.isHTTP()) {
-        LOG(NetworkCache, "(NetworkProcess) not HTTP");
+    if (!originalRequest.url().protocolIsInHTTPFamily() || !response.isHTTP())
         return StoreDecision::NoDueToProtocol;
-    }
-    if (originalRequest.httpMethod() != "GET") {
-        LOG(NetworkCache, "(NetworkProcess) method %s", originalRequest.httpMethod().utf8().data());
+
+    if (originalRequest.httpMethod() != "GET")
         return StoreDecision::NoDueToHTTPMethod;
-    }
+
+    auto requestDirectives = WebCore::parseCacheControlDirectives(originalRequest.httpHeaderFields());
+    if (requestDirectives.noStore)
+        return StoreDecision::NoDueToNoStoreRequest;
+
+    if (response.cacheControlContainsNoStore())
+        return StoreDecision::NoDueToNoStoreResponse;
 
     switch (response.httpStatusCode()) {
     case 200: // OK
@@ -284,10 +303,6 @@ static StoreDecision canStore(const WebCore::ResourceRequest& originalRequest, c
     case 307: // Temporary Redirect
     case 404: // Not Found
     case 410: // Gone
-        if (response.cacheControlContainsNoStore()) {
-            LOG(NetworkCache, "(NetworkProcess) Cache-control:no-store");
-            return StoreDecision::NoDueToNoStoreResponse;
-        }
         return StoreDecision::Yes;
     default:
         LOG(NetworkCache, "(NetworkProcess) status code %d", response.httpStatusCode());
@@ -305,7 +320,7 @@ void Cache::store(const WebCore::ResourceRequest& originalRequest, const WebCore
 
     StoreDecision storeDecision = canStore(originalRequest, response);
     if (storeDecision != StoreDecision::Yes) {
-        LOG(NetworkCache, "(NetworkProcess) didn't store");
+        LOG(NetworkCache, "(NetworkProcess) didn't store, storeDecision=%d", storeDecision);
         if (m_statistics) {
             auto key = makeCacheKey(originalRequest);
             m_statistics->recordNotCachingResponse(key, storeDecision);
index 335d61f..402fbdf 100644 (file)
@@ -68,7 +68,8 @@ enum class StoreDecision {
     NoDueToHTTPMethod,
     NoDueToAttachmentResponse,
     NoDueToNoStoreResponse,
-    NoDueToHTTPStatusCode
+    NoDueToHTTPStatusCode,
+    NoDueToNoStoreRequest
 };
 
 enum class UseDecision {
index 782d04a..d39fc9e 100644 (file)
@@ -236,6 +236,7 @@ static String storeDecisionToDiagnosticKey(StoreDecision storeDecision)
     case StoreDecision::NoDueToAttachmentResponse:
         return WebCore::DiagnosticLoggingKeys::isAttachmentKey();
     case StoreDecision::NoDueToNoStoreResponse:
+    case StoreDecision::NoDueToNoStoreRequest:
         return WebCore::DiagnosticLoggingKeys::cacheControlNoStoreKey();
     case StoreDecision::NoDueToHTTPStatusCode:
         return WebCore::DiagnosticLoggingKeys::uncacheableStatusCodeKey();