[webkit-changes] [201800] trunk/Source/WebCore

Wed, 08 Jun 2016 02:02:04 -0700

Title: [201800] trunk/Source/WebCore
Revision
201800
Author
an...@apple.com
Date
2016-06-08 02:01:07 -0700 (Wed, 08 Jun 2016)

Log 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:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (201799 => 201800)


--- trunk/Source/WebCore/ChangeLog	2016-06-08 08:00:22 UTC (rev 201799)
+++ trunk/Source/WebCore/ChangeLog	2016-06-08 09:01:07 UTC (rev 201800)
@@ -1,3 +1,53 @@
+2016-06-06  Antti Koivisto  <an...@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  <mmaxfi...@apple.com>
 
         Extend CSSFontSelector's lifetime to be longer than the Document's lifetime

Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (201799 => 201800)


--- trunk/Source/WebCore/loader/cache/CachedResource.cpp	2016-06-08 08:00:22 UTC (rev 201799)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp	2016-06-08 09:01:07 UTC (rev 201800)
@@ -182,7 +182,7 @@
     error(CachedResource::LoadError);
 }
 
-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
     // the Content-Security-Policy at the CachedResourceLoader layer so we can
@@ -191,24 +191,30 @@
     FrameLoader& frameLoader = cachedResourceLoader.frame()->loader();
     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);
+        request.setHTTPReferrer(outgoingReferrer);
+    FrameLoader::addHTTPOriginIfNeeded(request, outgoingOrigin);
 
-    frameLoader.addExtraFieldsToSubresourceRequest(m_resourceRequest);
+    frameLoader.addExtraFieldsToSubresourceRequest(request);
 }
 
+void CachedResource::addAdditionalRequestHeaders(CachedResourceLoader& cachedResourceLoader)
+{
+    addAdditionalRequestHeadersToRequest(m_resourceRequest, cachedResourceLoader);
+}
+
 void CachedResource::load(CachedResourceLoader& cachedResourceLoader, const ResourceLoaderOptions& options)
 {
     if (!cachedResourceLoader.frame()) {
@@ -417,6 +423,8 @@
     m_response = response;
     m_response.setType(m_responseType);
     m_response.setRedirected(m_redirectChainCacheStatus.status != RedirectChainCacheStatus::NoRedirection);
+
+    m_varyingHeaderValues = collectVaryingRequestHeaders(m_resourceRequest, m_response, m_sessionID);
 }
 
 void CachedResource::responseReceived(const ResourceResponse& response)
@@ -765,6 +773,17 @@
     return WebCore::redirectChainAllowsReuse(m_redirectChainCacheStatus, reuseExpiredRedirection);
 }
 
+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
 {
     static const int kAverageClientsHashMapSize = 384;

Modified: trunk/Source/WebCore/loader/cache/CachedResource.h (201799 => 201800)


--- trunk/Source/WebCore/loader/cache/CachedResource.h	2016-06-08 08:00:22 UTC (rev 201799)
+++ trunk/Source/WebCore/loader/cache/CachedResource.h	2016-06-08 09:01:07 UTC (rev 201800)
@@ -242,6 +242,8 @@
     virtual RevalidationDecision makeRevalidationDecision(CachePolicy) const;
     bool redirectChainAllowsReuse(ReuseExpiredRedirectionOrNot) const;
 
+    bool varyHeaderValuesMatch(const ResourceRequest&, const CachedResourceLoader&);
+
     bool isCacheValidator() const { return m_resourceToRevalidate; }
     CachedResource* resourceToRevalidate() const { return m_resourceToRevalidate; }
     
@@ -354,6 +356,8 @@
 
     RedirectChainCacheStatus m_redirectChainCacheStatus;
 
+    Vector<std::pair<String, String>> m_varyingHeaderValues;
+
     unsigned long m_identifierForLoadWithoutResourceLoader { 0 };
 };

Modified: trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp (201799 => 201800)


--- trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp	2016-06-08 08:00:22 UTC (rev 201799)
+++ trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp	2016-06-08 09:01:07 UTC (rev 201800)
@@ -753,6 +753,9 @@
         return Reload;
     }
 
+    if (!existingResource->varyHeaderValuesMatch(request, *this))
+        return Reload;
+
     auto* textDecoder = existingResource->textResourceDecoder();
     if (textDecoder && !textDecoder->hasEqualEncodingForCharset(cachedResourceRequest.charset()))
         return Reload;

Modified: trunk/Source/WebCore/platform/network/CacheValidation.cpp (201799 => 201800)


--- trunk/Source/WebCore/platform/network/CacheValidation.cpp	2016-06-08 08:00:22 UTC (rev 201799)
+++ trunk/Source/WebCore/platform/network/CacheValidation.cpp	2016-06-08 09:01:07 UTC (rev 201800)
@@ -27,6 +27,9 @@
 #include "CacheValidation.h"
 
 #include "HTTPHeaderMap.h"
+#include "NetworkStorageSession.h"
+#include "PlatformCookieJar.h"
+#include "ResourceRequest.h"
 #include "ResourceResponse.h"
 #include <wtf/CurrentTime.h>
 
@@ -326,4 +329,54 @@
     return result;
 }
 
+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;
+}
+
+}

Modified: trunk/Source/WebCore/platform/network/CacheValidation.h (201799 => 201800)


--- trunk/Source/WebCore/platform/network/CacheValidation.h	2016-06-08 08:00:22 UTC (rev 201799)
+++ trunk/Source/WebCore/platform/network/CacheValidation.h	2016-06-08 09:01:07 UTC (rev 201800)
@@ -27,11 +27,15 @@
 #define CacheValidation_h
 
 #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;
 
 struct RedirectChainCacheStatus {
@@ -65,6 +69,9 @@
 };
 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());
+
 }
 
 #endif

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to