CachedResourceLoader should set headers of the HTTP request prior checking for the...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Oct 2016 14:16:15 +0000 (14:16 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Oct 2016 14:16:15 +0000 (14:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163103

Patch by Youenn Fablet <youenn@apple.com> on 2016-10-25
Reviewed by Darin Adler.

No expected change of behavior.

Moved referrer, user-agent, and origin headers setting to CachedResourceRequest/CachedResourceLoader before checking the cache.
This allows simplifying vary header checks and is more inline with the fetch specification.

To compute the referrer value, we need to know whether the request is cross-origin.
A helper function isRequestCrossOrigin is added for that purpose and is also used in CachedResource to set its initial response tainting.

We should disable setting user-agent and origin headers by FrameLoader for subresources since this is now done in CachedResourceLoader.
This could be done as a follow-up patch.

* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::CachedResource):
(WebCore::CachedResource::load):
(WebCore::CachedResource::varyHeaderValuesMatch):
(WebCore::addAdditionalRequestHeadersToRequest): Deleted.
(WebCore::CachedResource::addAdditionalRequestHeaders): Deleted.
* loader/cache/CachedResource.h:
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::updateHTTPRequestHeaders):
(WebCore::CachedResourceLoader::requestResource):
(WebCore::CachedResourceLoader::determineRevalidationPolicy):
* loader/cache/CachedResourceLoader.h:
* loader/cache/CachedResourceRequest.cpp:
(WebCore::CachedResourceRequest::updateForAccessControl):
(WebCore::CachedResourceRequest::updateReferrerOriginAndUserAgentHeaders):
(WebCore::isRequestCrossOrigin):
* loader/cache/CachedResourceRequest.h:
(WebCore::CachedResourceRequest::setOrigin):

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

Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CachedResource.cpp
Source/WebCore/loader/cache/CachedResource.h
Source/WebCore/loader/cache/CachedResourceLoader.cpp
Source/WebCore/loader/cache/CachedResourceLoader.h
Source/WebCore/loader/cache/CachedResourceRequest.cpp
Source/WebCore/loader/cache/CachedResourceRequest.h

index 8834502..2c5d216 100644 (file)
@@ -1,3 +1,40 @@
+2016-10-25  Youenn Fablet  <youenn@apple.com>
+
+        CachedResourceLoader should set headers of the HTTP request prior checking for the cache
+        https://bugs.webkit.org/show_bug.cgi?id=163103
+
+        Reviewed by Darin Adler.
+
+        No expected change of behavior.
+
+        Moved referrer, user-agent, and origin headers setting to CachedResourceRequest/CachedResourceLoader before checking the cache.
+        This allows simplifying vary header checks and is more inline with the fetch specification.
+
+        To compute the referrer value, we need to know whether the request is cross-origin.
+        A helper function isRequestCrossOrigin is added for that purpose and is also used in CachedResource to set its initial response tainting.
+
+        We should disable setting user-agent and origin headers by FrameLoader for subresources since this is now done in CachedResourceLoader.
+        This could be done as a follow-up patch.
+
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::CachedResource):
+        (WebCore::CachedResource::load):
+        (WebCore::CachedResource::varyHeaderValuesMatch):
+        (WebCore::addAdditionalRequestHeadersToRequest): Deleted.
+        (WebCore::CachedResource::addAdditionalRequestHeaders): Deleted.
+        * loader/cache/CachedResource.h:
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::updateHTTPRequestHeaders):
+        (WebCore::CachedResourceLoader::requestResource):
+        (WebCore::CachedResourceLoader::determineRevalidationPolicy):
+        * loader/cache/CachedResourceLoader.h:
+        * loader/cache/CachedResourceRequest.cpp:
+        (WebCore::CachedResourceRequest::updateForAccessControl):
+        (WebCore::CachedResourceRequest::updateReferrerOriginAndUserAgentHeaders):
+        (WebCore::isRequestCrossOrigin):
+        * loader/cache/CachedResourceRequest.h:
+        (WebCore::CachedResourceRequest::setOrigin):
+
 2016-10-25  Andreas Kling  <akling@apple.com>
 
         More PassRefPtr purging in WebCore.
index 5b75576..e07938c 100644 (file)
@@ -47,7 +47,6 @@
 #include "ResourceHandle.h"
 #include "SchemeRegistry.h"
 #include "SecurityOrigin.h"
-#include "SecurityPolicy.h"
 #include "SubresourceLoader.h"
 #include <wtf/CurrentTime.h>
 #include <wtf/MathExtras.h>
@@ -135,9 +134,7 @@ CachedResource::CachedResource(CachedResourceRequest&& request, Type type, Sessi
     // FIXME: We should have a better way of checking for Navigation loads, maybe FetchMode::Options::Navigate.
     ASSERT(m_origin || m_type == CachedResource::MainResource);
 
-    if (m_options.mode != FetchOptions::Mode::SameOrigin && m_origin
-        && !(m_resourceRequest.url().protocolIsData() && m_options.sameOriginDataURLFlag == SameOriginDataURLFlag::Set)
-        && !m_origin->canRequest(m_resourceRequest.url()))
+    if (isRequestCrossOrigin(m_origin.get(), m_resourceRequest.url(), m_options))
         setCrossOrigin();
 }
 
@@ -183,66 +180,6 @@ void CachedResource::failBeforeStarting()
     error(CachedResource::LoadError);
 }
 
-static void addAdditionalRequestHeadersToRequest(ResourceRequest& request, const CachedResourceLoader& cachedResourceLoader, CachedResource& resource)
-{
-    if (resource.type() == CachedResource::MainResource)
-        return;
-    // In some cases we may try to load resources in frameless documents. Such loads always fail.
-    // FIXME: We shouldn't get this far.
-    if (!cachedResourceLoader.frame())
-        return;
-
-    // Note: We skip the Content-Security-Policy check here because we check
-    // the Content-Security-Policy at the CachedResourceLoader layer so we can
-    // handle different resource types differently.
-    FrameLoader& frameLoader = cachedResourceLoader.frame()->loader();
-    String outgoingReferrer;
-    String outgoingOrigin;
-    if (request.httpReferrer().isNull()) {
-        outgoingReferrer = frameLoader.outgoingReferrer();
-        outgoingOrigin = frameLoader.outgoingOrigin();
-    } else {
-        outgoingReferrer = request.httpReferrer();
-        outgoingOrigin = SecurityOrigin::createFromString(outgoingReferrer)->toString();
-    }
-
-    // FIXME: Refactor SecurityPolicy::generateReferrerHeader to align with new terminology used in https://w3c.github.io/webappsec-referrer-policy.
-    switch (resource.options().referrerPolicy) {
-    case FetchOptions::ReferrerPolicy::EmptyString: {
-        ReferrerPolicy referrerPolicy = cachedResourceLoader.document() ? cachedResourceLoader.document()->referrerPolicy() : ReferrerPolicy::Default;
-        outgoingReferrer = SecurityPolicy::generateReferrerHeader(referrerPolicy, request.url(), outgoingReferrer);
-        break; }
-    case FetchOptions::ReferrerPolicy::NoReferrerWhenDowngrade:
-        outgoingReferrer = SecurityPolicy::generateReferrerHeader(ReferrerPolicy::Default, request.url(), outgoingReferrer);
-        break;
-    case FetchOptions::ReferrerPolicy::NoReferrer:
-        outgoingReferrer = String();
-        break;
-    case FetchOptions::ReferrerPolicy::Origin:
-        outgoingReferrer = SecurityPolicy::generateReferrerHeader(ReferrerPolicy::Origin, request.url(), outgoingReferrer);
-        break;
-    case FetchOptions::ReferrerPolicy::OriginWhenCrossOrigin:
-        if (resource.isCrossOrigin())
-            outgoingReferrer = SecurityPolicy::generateReferrerHeader(ReferrerPolicy::Origin, request.url(), outgoingReferrer);
-        break;
-    case FetchOptions::ReferrerPolicy::UnsafeUrl:
-        break;
-    };
-
-    if (outgoingReferrer.isEmpty())
-        request.clearHTTPReferrer();
-    else
-        request.setHTTPReferrer(outgoingReferrer);
-    FrameLoader::addHTTPOriginIfNeeded(request, outgoingOrigin);
-
-    frameLoader.addExtraFieldsToSubresourceRequest(request);
-}
-
-void CachedResource::addAdditionalRequestHeaders(CachedResourceLoader& loader)
-{
-    addAdditionalRequestHeadersToRequest(m_resourceRequest, loader, *this);
-}
-
 void CachedResource::load(CachedResourceLoader& cachedResourceLoader)
 {
     if (!cachedResourceLoader.frame()) {
@@ -311,7 +248,11 @@ void CachedResource::load(CachedResourceLoader& cachedResourceLoader)
 #endif
     m_resourceRequest.setPriority(loadPriority());
 
-    addAdditionalRequestHeaders(cachedResourceLoader);
+    // Navigation algorithm is setting up the request before sending it to CachedResourceLoader?CachedResource.
+    // So no need for extra fields for MainResource.
+    if (type() != CachedResource::MainResource)
+        frameLoader.addExtraFieldsToSubresourceRequest(m_resourceRequest);
+
 
     // FIXME: It's unfortunate that the cache layer and below get to know anything about fragment identifiers.
     // We should look into removing the expectation of that knowledge from the platform network stacks.
@@ -835,15 +776,12 @@ bool CachedResource::redirectChainAllowsReuse(ReuseExpiredRedirectionOrNot reuse
     return WebCore::redirectChainAllowsReuse(m_redirectChainCacheStatus, reuseExpiredRedirection);
 }
 
-bool CachedResource::varyHeaderValuesMatch(const ResourceRequest& request, const CachedResourceLoader& cachedResourceLoader)
+bool CachedResource::varyHeaderValuesMatch(const ResourceRequest& request)
 {
     if (m_varyingHeaderValues.isEmpty())
         return true;
 
-    ResourceRequest requestWithFullHeaders(request);
-    addAdditionalRequestHeadersToRequest(requestWithFullHeaders, cachedResourceLoader, *this);
-
-    return verifyVaryingRequestHeaders(m_varyingHeaderValues, requestWithFullHeaders, m_sessionID);
+    return verifyVaryingRequestHeaders(m_varyingHeaderValues, request, m_sessionID);
 }
 
 unsigned CachedResource::overheadSize() const
index e95749e..15c3803 100644 (file)
@@ -233,8 +233,8 @@ public:
     void increasePreloadCount() { ++m_preloadCount; }
     void decreasePreloadCount() { ASSERT(m_preloadCount); --m_preloadCount; }
 
-    void registerHandle(CachedResourceHandleBase* h);
-    WEBCORE_EXPORT void unregisterHandle(CachedResourceHandleBase* h);
+    void registerHandle(CachedResourceHandleBase*);
+    WEBCORE_EXPORT void unregisterHandle(CachedResourceHandleBase*);
 
     bool canUseCacheValidator() const;
 
@@ -243,7 +243,7 @@ public:
     bool redirectChainAllowsReuse(ReuseExpiredRedirectionOrNot) const;
     bool hasRedirections() const { return m_redirectChainCacheStatus.status != RedirectChainCacheStatus::Status::NoRedirection;  }
 
-    bool varyHeaderValuesMatch(const ResourceRequest&, const CachedResourceLoader&);
+    bool varyHeaderValuesMatch(const ResourceRequest&);
 
     bool isCacheValidator() const { return m_resourceToRevalidate; }
     CachedResource* resourceToRevalidate() const { return m_resourceToRevalidate; }
index d832de8..40b30b4 100644 (file)
@@ -65,6 +65,7 @@
 #include "RuntimeEnabledFeatures.h"
 #include "ScriptController.h"
 #include "SecurityOrigin.h"
+#include "SecurityPolicy.h"
 #include "SessionID.h"
 #include "Settings.h"
 #include "StyleSheetContents.h"
@@ -662,10 +663,18 @@ void CachedResourceLoader::prepareFetch(CachedResource::Type type, CachedResourc
     // FIXME: Decide whether to support client hints
 }
 
-
-void CachedResourceLoader::updateHTTPRequestHeaders(CachedResourceRequest& request)
+void CachedResourceLoader::updateHTTPRequestHeaders(CachedResource::Type type, CachedResourceRequest& request)
 {
-    // Implementing steps 10 to 12 of https://fetch.spec.whatwg.org/#http-network-or-cache-fetch
+    // Implementing steps 7 to 12 of https://fetch.spec.whatwg.org/#http-network-or-cache-fetch
+
+    // FIXME: We should reconcile handling of MainResource with other resources.
+    if (type != CachedResource::Type::MainResource) {
+        // In some cases we may try to load resources in frameless documents. Such loads always fail.
+        // FIXME: We shouldn't need to do the check on frame.
+        if (auto* frame = this->frame())
+            request.updateReferrerOriginAndUserAgentHeaders(frame->loader(), document() ? document()->referrerPolicy() : ReferrerPolicy::Default);
+    }
+
     request.updateAccordingCacheMode();
 }
 
@@ -724,7 +733,7 @@ CachedResourceHandle<CachedResource> CachedResourceLoader::requestResource(Cache
 #endif
 
     if (request.resourceRequest().url().protocolIsInHTTPFamily())
-        updateHTTPRequestHeaders(request);
+        updateHTTPRequestHeaders(type, request);
 
     auto& memoryCache = MemoryCache::singleton();
     if (request.allowsCaching() && memoryCache.disabled()) {
@@ -921,7 +930,7 @@ CachedResourceLoader::RevalidationPolicy CachedResourceLoader::determineRevalida
         return Reload;
     }
 
-    if (!existingResource->varyHeaderValuesMatch(request, *this))
+    if (!existingResource->varyHeaderValuesMatch(request))
         return Reload;
 
     auto* textDecoder = existingResource->textResourceDecoder();
index f0b5f2f..7b471d8 100644 (file)
@@ -154,13 +154,15 @@ private:
     enum class ForPreload { Yes, No };
     enum class DeferOption { NoDefer, DeferredByClient };
 
-    void updateHTTPRequestHeaders(CachedResourceRequest&);
     CachedResourceHandle<CachedResource> requestResource(CachedResource::Type, CachedResourceRequest&&, ForPreload = ForPreload::No, DeferOption = DeferOption::NoDefer);
-    void prepareFetch(CachedResource::Type, CachedResourceRequest&);
     CachedResourceHandle<CachedResource> revalidateResource(CachedResourceRequest&&, CachedResource&);
     CachedResourceHandle<CachedResource> loadResource(CachedResource::Type, CachedResourceRequest&&);
     CachedResourceHandle<CachedResource> requestPreload(CachedResource::Type, CachedResourceRequest&&);
 
+    void prepareFetch(CachedResource::Type, CachedResourceRequest&);
+    void updateHTTPRequestHeaders(CachedResource::Type, CachedResourceRequest&);
+    void updateReferrerOriginAndUserAgentHeaders(CachedResourceRequest&);
+
     bool canRequest(CachedResource::Type, const URL&, const CachedResourceRequest&, ForPreload);
 
     enum RevalidationPolicy { Use, Revalidate, Reload, Load };
index bacca19..6ce8aab 100644 (file)
 #include "CrossOriginAccessControl.h"
 #include "Document.h"
 #include "Element.h"
+#include "FrameLoader.h"
 #include "HTTPHeaderValues.h"
 #include "MemoryCache.h"
+#include "SecurityPolicy.h"
 #include <wtf/NeverDestroyed.h>
 
 namespace WebCore {
@@ -102,7 +104,7 @@ void CachedResourceRequest::updateForAccessControl(Document& document)
     ASSERT(document.securityOrigin());
 
     m_origin = document.securityOrigin();
-    WebCore::updateRequestForAccessControl(m_resourceRequest, *document.securityOrigin(), m_options.allowCredentials);
+    WebCore::updateRequestForAccessControl(m_resourceRequest, *m_origin, m_options.allowCredentials);
 }
 
 void upgradeInsecureResourceRequestIfNeeded(ResourceRequest& request, Document& document)
@@ -209,4 +211,64 @@ void CachedResourceRequest::applyBlockedStatus(const ContentExtensions::BlockedS
 }
 #endif
 
+void CachedResourceRequest::updateReferrerOriginAndUserAgentHeaders(FrameLoader& frameLoader, ReferrerPolicy defaultPolicy)
+{
+    // Implementing step 7 to 9 of https://fetch.spec.whatwg.org/#http-network-or-cache-fetch
+
+    String outgoingOrigin;
+    String outgoingReferrer = m_resourceRequest.httpReferrer();
+    if (!outgoingReferrer.isNull())
+        outgoingOrigin = SecurityOrigin::createFromString(outgoingReferrer)->toString();
+    else {
+        outgoingReferrer = frameLoader.outgoingReferrer();
+        outgoingOrigin = frameLoader.outgoingOrigin();
+    }
+
+    // FIXME: Refactor SecurityPolicy::generateReferrerHeader to align with new terminology used in https://w3c.github.io/webappsec-referrer-policy.
+    switch (m_options.referrerPolicy) {
+    case FetchOptions::ReferrerPolicy::EmptyString: {
+        outgoingReferrer = SecurityPolicy::generateReferrerHeader(defaultPolicy, m_resourceRequest.url(), outgoingReferrer);
+        break; }
+    case FetchOptions::ReferrerPolicy::NoReferrerWhenDowngrade:
+        outgoingReferrer = SecurityPolicy::generateReferrerHeader(ReferrerPolicy::Default, m_resourceRequest.url(), outgoingReferrer);
+        break;
+    case FetchOptions::ReferrerPolicy::NoReferrer:
+        outgoingReferrer = String();
+        break;
+    case FetchOptions::ReferrerPolicy::Origin:
+        outgoingReferrer = SecurityPolicy::generateReferrerHeader(ReferrerPolicy::Origin, m_resourceRequest.url(), outgoingReferrer);
+        break;
+    case FetchOptions::ReferrerPolicy::OriginWhenCrossOrigin:
+        if (isRequestCrossOrigin(m_origin.get(), m_resourceRequest.url(), m_options))
+            outgoingReferrer = SecurityPolicy::generateReferrerHeader(ReferrerPolicy::Origin, m_resourceRequest.url(), outgoingReferrer);
+        break;
+    case FetchOptions::ReferrerPolicy::UnsafeUrl:
+        break;
+    };
+
+    if (outgoingReferrer.isEmpty())
+        m_resourceRequest.clearHTTPReferrer();
+    else
+        m_resourceRequest.setHTTPReferrer(outgoingReferrer);
+    FrameLoader::addHTTPOriginIfNeeded(m_resourceRequest, outgoingOrigin);
+
+    frameLoader.applyUserAgent(m_resourceRequest);
+}
+
+bool isRequestCrossOrigin(SecurityOrigin* origin, const URL& requestURL, const ResourceLoaderOptions& options)
+{
+    if (!origin)
+        return false;
+
+    // Using same origin mode guarantees the loader will not do a cross-origin load, so we let it take care of it and just return false.
+    if (options.mode == FetchOptions::Mode::SameOrigin)
+        return false;
+
+    // FIXME: We should remove options.sameOriginDataURLFlag once https://github.com/whatwg/fetch/issues/393 is fixed.
+    if (requestURL.protocolIsData() && options.sameOriginDataURLFlag == SameOriginDataURLFlag::Set)
+        return false;
+
+    return !origin->canRequest(requestURL);
+}
+
 } // namespace WebCore
index 54fc8d8..d826331 100644 (file)
@@ -41,7 +41,12 @@ namespace WebCore {
 namespace ContentExtensions {
 struct BlockedStatus;
 }
+
 class Document;
+class FrameLoader;
+enum class ReferrerPolicy;
+
+bool isRequestCrossOrigin(SecurityOrigin*, const URL& requestURL, const ResourceLoaderOptions&);
 
 class CachedResourceRequest {
 public:
@@ -63,6 +68,7 @@ public:
     void setAsPotentiallyCrossOrigin(const String&, Document&);
     void updateForAccessControl(Document&);
 
+    void updateReferrerOriginAndUserAgentHeaders(FrameLoader&, ReferrerPolicy);
     void upgradeInsecureRequestIfNeeded(Document&);
     void setAcceptHeaderIfNone(CachedResource::Type);
     void updateAccordingCacheMode();
@@ -74,7 +80,7 @@ public:
     void setDomainForCachePartition(Document&);
 #endif
 
-    void setOrigin(RefPtr<SecurityOrigin>&& origin) { ASSERT(!m_origin); m_origin = WTFMove(origin); }
+    void setOrigin(RefPtr<SecurityOrigin>&& origin) { m_origin = WTFMove(origin); }
     RefPtr<SecurityOrigin> releaseOrigin() { return WTFMove(m_origin); }
     SecurityOrigin* origin() const { return m_origin.get(); }