Changeset 201800 in webkit

Timestamp:

Jun 8, 2016 2:01:07 AM (8 years ago)

Author:

Antti Koivisto

Message:

WebKit memory cache doesn't respect Vary header
​https://bugs.webkit.org/show_bug.cgi?id=71509
<rdar://problem/26651033>

Reviewed by Sam Weinig.

Implement Vary header support in WebCore memory cache.

The patch moves Vary header code from WebKit2 Network Cache to WebCore and uses it to
verify the headers for CachedResources.

• loader/cache/CachedResource.cpp:

(WebCore::CachedResource::failBeforeStarting):
(WebCore::addAdditionalRequestHeadersToRequest):

Factor into standalone function so we can use it from varyHeaderValuesMatch.

(WebCore::CachedResource::addAdditionalRequestHeaders):
(WebCore::CachedResource::load):
(WebCore::CachedResource::setResponse):

Collect the Vary header values when we receive a response.

(WebCore::CachedResource::responseReceived):
(WebCore::CachedResource::redirectChainAllowsReuse):
(WebCore::CachedResource::varyHeaderValuesMatch):

Test for Vary match.

(WebCore::CachedResource::overheadSize):

• loader/cache/CachedResource.h:

(WebCore::CachedResource::isCacheValidator):
(WebCore::CachedResource::resourceToRevalidate):

• loader/cache/CachedResourceLoader.cpp:

(WebCore::CachedResourceLoader::determineRevalidationPolicy):

Reload on Vary mismatch.

• platform/network/CacheValidation.cpp:

(WebCore::parseCacheControlDirectives):
(WebCore::headerValueForVary):
(WebCore::collectVaryingRequestHeaders):
(WebCore::verifyVaryingRequestHeaders):

Vary header collection and validation code moves here.

• platform/network/CacheValidation.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/cache/CachedResource.cpp (modified) (4 diffs)
• loader/cache/CachedResource.h (modified) (2 diffs)
• loader/cache/CachedResourceLoader.cpp (modified) (1 diff)
• platform/network/CacheValidation.cpp (modified) (2 diffs)
• platform/network/CacheValidation.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-06-06  Antti Koivisto  <antti@apple.com>
+
+        WebKit memory cache doesn't respect Vary header
+        https://bugs.webkit.org/show_bug.cgi?id=71509
+        <rdar://problem/26651033>
+
+        Reviewed by Sam Weinig.
+
+        Implement Vary header support in WebCore memory cache.
+
+        The patch moves Vary header code from WebKit2 Network Cache to WebCore and uses it to
+        verify the headers for CachedResources.
+
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::failBeforeStarting):
+        (WebCore::addAdditionalRequestHeadersToRequest):
+
+            Factor into standalone function so we can use it from varyHeaderValuesMatch.
+
+        (WebCore::CachedResource::addAdditionalRequestHeaders):
+        (WebCore::CachedResource::load):
+        (WebCore::CachedResource::setResponse):
+
+            Collect the Vary header values when we receive a response.
+
+        (WebCore::CachedResource::responseReceived):
+        (WebCore::CachedResource::redirectChainAllowsReuse):
+        (WebCore::CachedResource::varyHeaderValuesMatch):
+
+            Test for Vary match.
+
+        (WebCore::CachedResource::overheadSize):
+        * loader/cache/CachedResource.h:
+        (WebCore::CachedResource::isCacheValidator):
+        (WebCore::CachedResource::resourceToRevalidate):
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::determineRevalidationPolicy):
+
+            Reload on Vary mismatch.
+
+        * platform/network/CacheValidation.cpp:
+        (WebCore::parseCacheControlDirectives):
+        (WebCore::headerValueForVary):
+        (WebCore::collectVaryingRequestHeaders):
+        (WebCore::verifyVaryingRequestHeaders):
+
+            Vary header collection and validation code moves here.
+
+        * platform/network/CacheValidation.h:
+
 2016-06-08  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/loader/cache/CachedResource.cpp

@@ -183 +183 @@
 }
 
-void CachedResource::addAdditionalRequestHeaders(CachedResourceLoader& cachedResourceLoader)
+static void addAdditionalRequestHeadersToRequest(ResourceRequest& request, const CachedResourceLoader& cachedResourceLoader)
 {
     // Note: We skip the Content-Security-Policy check here because we check
@@ -192 +192 @@
     String outgoingReferrer;
     String outgoingOrigin;
-    if (m_resourceRequest.httpReferrer().isNull()) {
+    if (request.httpReferrer().isNull()) {
         outgoingReferrer = frameLoader.outgoingReferrer();
         outgoingOrigin = frameLoader.outgoingOrigin();
     } else {
-        outgoingReferrer = m_resourceRequest.httpReferrer();
+        outgoingReferrer = request.httpReferrer();
         outgoingOrigin = SecurityOrigin::createFromString(outgoingReferrer)->toString();
     }
 
-    outgoingReferrer = SecurityPolicy::generateReferrerHeader(cachedResourceLoader.document()->referrerPolicy(), m_resourceRequest.url(), outgoingReferrer);
+    auto referrerPolicy = cachedResourceLoader.document() ? cachedResourceLoader.document()->referrerPolicy() : ReferrerPolicy::Default;
+    outgoingReferrer = SecurityPolicy::generateReferrerHeader(referrerPolicy, request.url(), outgoingReferrer);
     if (outgoingReferrer.isEmpty())
-        m_resourceRequest.clearHTTPReferrer();
+        request.clearHTTPReferrer();
     else
-        m_resourceRequest.setHTTPReferrer(outgoingReferrer);
-    FrameLoader::addHTTPOriginIfNeeded(m_resourceRequest, outgoingOrigin);
-
-    frameLoader.addExtraFieldsToSubresourceRequest(m_resourceRequest);
+        request.setHTTPReferrer(outgoingReferrer);
+    FrameLoader::addHTTPOriginIfNeeded(request, outgoingOrigin);
+
+    frameLoader.addExtraFieldsToSubresourceRequest(request);
+}
+
+void CachedResource::addAdditionalRequestHeaders(CachedResourceLoader& cachedResourceLoader)
+{
+    addAdditionalRequestHeadersToRequest(m_resourceRequest, cachedResourceLoader);
 }
 
@@ -418 +424 @@
     m_response.setType(m_responseType);
     m_response.setRedirected(m_redirectChainCacheStatus.status != RedirectChainCacheStatus::NoRedirection);
+
+    m_varyingHeaderValues = collectVaryingRequestHeaders(m_resourceRequest, m_response, m_sessionID);
 }
 
@@ -766 +774 @@
 }
 
+bool CachedResource::varyHeaderValuesMatch(const ResourceRequest& request, const CachedResourceLoader& cachedResourceLoader)
+{
+    if (m_varyingHeaderValues.isEmpty())
+        return true;
+
+    ResourceRequest requestWithFullHeaders(request);
+    addAdditionalRequestHeadersToRequest(requestWithFullHeaders, cachedResourceLoader);
+
+    return verifyVaryingRequestHeaders(m_varyingHeaderValues, requestWithFullHeaders, m_sessionID);
+}
+
 unsigned CachedResource::overheadSize() const
 {

trunk/Source/WebCore/loader/cache/CachedResource.h

@@ -243 +243 @@
     bool redirectChainAllowsReuse(ReuseExpiredRedirectionOrNot) const;
 
+    bool varyHeaderValuesMatch(const ResourceRequest&, const CachedResourceLoader&);
+
     bool isCacheValidator() const { return m_resourceToRevalidate; }
     CachedResource* resourceToRevalidate() const { return m_resourceToRevalidate; }
@@ -355 +357 @@
     RedirectChainCacheStatus m_redirectChainCacheStatus;
 
+    Vector<std::pair<String, String>> m_varyingHeaderValues;
+
     unsigned long m_identifierForLoadWithoutResourceLoader { 0 };
 };

trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

@@ -754 +754 @@
     }
 
+    if (!existingResource->varyHeaderValuesMatch(request, *this))
+        return Reload;
+
     auto* textDecoder = existingResource->textResourceDecoder();
     if (textDecoder && !textDecoder->hasEqualEncodingForCharset(cachedResourceRequest.charset()))

trunk/Source/WebCore/platform/network/CacheValidation.cpp

@@ -28 +28 @@
 
 #include "HTTPHeaderMap.h"
+#include "NetworkStorageSession.h"
+#include "PlatformCookieJar.h"
+#include "ResourceRequest.h"
 #include "ResourceResponse.h"
 #include <wtf/CurrentTime.h>
@@ -327 +330 @@
 }
 
-}
+static String headerValueForVary(SessionID sessionID, const ResourceRequest& request, const String& headerName)
+{
+    // Explicit handling for cookies is needed because they are added magically by the networking layer.
+    // FIXME: The value might have changed between making the request and retrieving the cookie here.
+    // We could fetch the cookie when making the request but that seems overkill as the case is very rare and it
+    // is a blocking operation. This should be sufficient to cover reasonable cases.
+    if (headerName == httpHeaderNameString(HTTPHeaderName::Cookie)) {
+        if (sessionID != SessionID::defaultSessionID()) {
+            // FIXME: Don't know how to get the cookie. There should be a global way to get NetworkStorageSession from sessionID.
+            return "";
+        }
+        return cookieRequestHeaderFieldValue(NetworkStorageSession::defaultStorageSession(), request.firstPartyForCookies(), request.url());
+    }
+    return request.httpHeaderField(headerName);
+}
+
+Vector<std::pair<String, String>> collectVaryingRequestHeaders(const WebCore::ResourceRequest& request, const WebCore::ResourceResponse& response, SessionID sessionID)
+{
+    String varyValue = response.httpHeaderField(WebCore::HTTPHeaderName::Vary);
+    if (varyValue.isEmpty())
+        return { };
+    Vector<String> varyingHeaderNames;
+    varyValue.split(',', /*allowEmptyEntries*/ false, varyingHeaderNames);
+    Vector<std::pair<String, String>> varyingRequestHeaders;
+    varyingRequestHeaders.reserveCapacity(varyingHeaderNames.size());
+    for (auto& varyHeaderName : varyingHeaderNames) {
+        String headerName = varyHeaderName.stripWhiteSpace();
+        String headerValue = headerValueForVary(sessionID, request, headerName);
+        varyingRequestHeaders.append(std::make_pair(headerName, headerValue));
+    }
+    return varyingRequestHeaders;
+}
+
+bool verifyVaryingRequestHeaders(const Vector<std::pair<String, String>>& varyingRequestHeaders, const WebCore::ResourceRequest& request, SessionID sessionID)
+{
+    for (auto& varyingRequestHeader : varyingRequestHeaders) {
+        // FIXME: Vary: * in response would ideally trigger a cache delete instead of a store.
+        if (varyingRequestHeader.first == "*")
+            return false;
+        if (sessionID != SessionID::defaultSessionID() && varyingRequestHeader.first == httpHeaderNameString(HTTPHeaderName::Cookie)) {
+            // FIXME: See the comment in headerValueForVary.
+            return false;
+        }
+        String headerValue = headerValueForVary(sessionID, request, varyingRequestHeader.first);
+        if (headerValue != varyingRequestHeader.second)
+            return false;
+    }
+    return true;
+}
+
+}

trunk/Source/WebCore/platform/network/CacheValidation.h

@@ -28 +28 @@
 
 #include "PlatformExportMacros.h"
+#include "SessionID.h"
 #include <wtf/Optional.h>
+#include <wtf/Vector.h>
+#include <wtf/text/WTFString.h>
 
 namespace WebCore {
 
 class HTTPHeaderMap;
+class ResourceRequest;
 class ResourceResponse;
 
@@ -66 +70 @@
 WEBCORE_EXPORT CacheControlDirectives parseCacheControlDirectives(const HTTPHeaderMap&);
 
+WEBCORE_EXPORT Vector<std::pair<String, String>> collectVaryingRequestHeaders(const ResourceRequest&, const ResourceResponse&, SessionID = SessionID::defaultSessionID());
+WEBCORE_EXPORT bool verifyVaryingRequestHeaders(const Vector<std::pair<String, String>>& varyingRequestHeaders, const ResourceRequest&, SessionID = SessionID::defaultSessionID());
+
 }