CachedResourceRequest should store a SecurityOrigin
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Sep 2016 08:57:12 +0000 (08:57 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 22 Sep 2016 08:57:12 +0000 (08:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=162258

Patch by Youenn Fablet <youenn@apple.com> on 2016-09-22
Reviewed by Sam Weinig.

Source/WebCore:

Test: http/tests/local/script-crossorigin-loads-file-scheme.html

Passing SecurityOrigin from loader clients to CachedResource through CachedResourceRequest.
This ensures that specific origin properties like universal access are well preserved.

* loader/DocumentThreadableLoader.cpp:
(WebCore::DocumentThreadableLoader::loadRequest): Set origin to the request.
* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::CachedResource): Setting origin from the request.
Computing CORS state based on that origin.
(WebCore::CachedResource::load): Removing origin computation.
(WebCore::CachedResource::loadFrom): Ditto.
(WebCore::CachedResource::computeOrigin): Deleted.
* loader/cache/CachedResource.h:
* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::updateCachedResourceWithCurrentRequest):
(WebCore::CachedResourceLoader::prepareFetch): Introduced to implement step 1 to 7 of https://fetch.spec.whatwg.org/#fetching.
(WebCore::CachedResourceLoader::requestResource):
* loader/cache/CachedResourceLoader.h:
* loader/cache/CachedResourceRequest.cpp:
(WebCore::CachedResourceRequest::setAsPotentiallyCrossOrigin): Storing origin.
* loader/cache/CachedResourceRequest.h:
(WebCore::CachedResourceRequest::setOrigin):
(WebCore::CachedResourceRequest::releaseOrigin):
(WebCore::CachedResourceRequest::origin):

LayoutTests:

Updated test to expect load even though CORS checks should fail as the document origin has universal access.

* http/tests/local/script-crossorigin-loads-fail-origin-expected.txt: Removed.
* http/tests/local/script-crossorigin-loads-file-scheme-expected.txt: Added.
* http/tests/local/script-crossorigin-loads-file-scheme.html: Renamed from LayoutTests/http/tests/local/script-crossorigin-loads-fail-origin.html.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/local/script-crossorigin-loads-fail-origin-expected.txt [deleted file]
LayoutTests/http/tests/local/script-crossorigin-loads-file-scheme-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/local/script-crossorigin-loads-file-scheme.html [moved from LayoutTests/http/tests/local/script-crossorigin-loads-fail-origin.html with 60% similarity]
Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentThreadableLoader.cpp
Source/WebCore/loader/cache/CachedResource.cpp
Source/WebCore/loader/cache/CachedResource.h
Source/WebCore/loader/cache/CachedResourceLoader.cpp
Source/WebCore/loader/cache/CachedResourceRequest.cpp
Source/WebCore/loader/cache/CachedResourceRequest.h

index 9c96e7c..f23d5f8 100644 (file)
@@ -1,3 +1,16 @@
+2016-09-22  Youenn Fablet  <youenn@apple.com>
+
+        CachedResourceRequest should store a SecurityOrigin
+        https://bugs.webkit.org/show_bug.cgi?id=162258
+
+        Reviewed by Sam Weinig.
+
+        Updated test to expect load even though CORS checks should fail as the document origin has universal access.
+
+        * http/tests/local/script-crossorigin-loads-fail-origin-expected.txt: Removed.
+        * http/tests/local/script-crossorigin-loads-file-scheme-expected.txt: Added.
+        * http/tests/local/script-crossorigin-loads-file-scheme.html: Renamed from LayoutTests/http/tests/local/script-crossorigin-loads-fail-origin.html.
+
 2016-09-19  Sergio Villar Senin  <svillar@igalia.com>
 
         [css-grid] Remove the x2 computation of row sizes with indefinite heights
diff --git a/LayoutTests/http/tests/local/script-crossorigin-loads-fail-origin-expected.txt b/LayoutTests/http/tests/local/script-crossorigin-loads-fail-origin-expected.txt
deleted file mode 100644 (file)
index fb0b830..0000000
+++ /dev/null
@@ -1,5 +0,0 @@
-CONSOLE MESSAGE: Origin  is not allowed by Access-Control-Allow-Origin.
-CONSOLE MESSAGE: Cross-origin script load denied by Cross-Origin Resource Sharing policy.
-This test fails if the script loads correctly.
-
-PASS
diff --git a/LayoutTests/http/tests/local/script-crossorigin-loads-file-scheme-expected.txt b/LayoutTests/http/tests/local/script-crossorigin-loads-file-scheme-expected.txt
new file mode 100644 (file)
index 0000000..f05bbf3
--- /dev/null
@@ -0,0 +1,4 @@
+ALERT: script ran.
+This test passes if the script loads correctly.
+
+PASS
@@ -1,5 +1,5 @@
 <body>
-<p>This test fails if the script loads correctly.</p>
+<p>This test passes if the script loads correctly.</p>
 <pre></pre>
 <script>
 if (window.testRunner) {
@@ -15,9 +15,10 @@ function done(msg) {
 
 var script = document.createElement("script");
 script.crossOrigin = "use-credentials";
-// We are serving the test from the filesystem, so it should fail as authorized origin is 127.0.0.1:8000.
+// We are serving the test from the filesystem and file URLs are granted universal access.
+// This bypasses CORS checks and will allow access to 127.0.0.1:8000.
 script.src = "http://localhost:8000/security/resources/cors-script.php?credentials=true";
-script.onload = function() { done("FAIL"); }
-script.onerror = function() { done("PASS");}
+script.onload = function() { done("PASS"); }
+script.onerror = function() { done("FAIL");}
 document.body.appendChild(script);
 </script>
index 376f3b4..c4a8165 100644 (file)
@@ -1,5 +1,38 @@
 2016-09-22  Youenn Fablet  <youenn@apple.com>
 
+        CachedResourceRequest should store a SecurityOrigin
+        https://bugs.webkit.org/show_bug.cgi?id=162258
+
+        Reviewed by Sam Weinig.
+
+        Test: http/tests/local/script-crossorigin-loads-file-scheme.html
+
+        Passing SecurityOrigin from loader clients to CachedResource through CachedResourceRequest.
+        This ensures that specific origin properties like universal access are well preserved.
+
+        * loader/DocumentThreadableLoader.cpp:
+        (WebCore::DocumentThreadableLoader::loadRequest): Set origin to the request.
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::CachedResource): Setting origin from the request.
+        Computing CORS state based on that origin.
+        (WebCore::CachedResource::load): Removing origin computation.
+        (WebCore::CachedResource::loadFrom): Ditto.
+        (WebCore::CachedResource::computeOrigin): Deleted.
+        * loader/cache/CachedResource.h:
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::updateCachedResourceWithCurrentRequest):
+        (WebCore::CachedResourceLoader::prepareFetch): Introduced to implement step 1 to 7 of https://fetch.spec.whatwg.org/#fetching.
+        (WebCore::CachedResourceLoader::requestResource):
+        * loader/cache/CachedResourceLoader.h:
+        * loader/cache/CachedResourceRequest.cpp:
+        (WebCore::CachedResourceRequest::setAsPotentiallyCrossOrigin): Storing origin.
+        * loader/cache/CachedResourceRequest.h:
+        (WebCore::CachedResourceRequest::setOrigin):
+        (WebCore::CachedResourceRequest::releaseOrigin):
+        (WebCore::CachedResourceRequest::origin):
+
+2016-09-22  Youenn Fablet  <youenn@apple.com>
+
         Refactor ContentSecurityPolicy::allow* methods
         https://bugs.webkit.org/show_bug.cgi?id=162335
 
index 5dde6e0..7ac25f4 100644 (file)
@@ -370,6 +370,7 @@ void DocumentThreadableLoader::loadRequest(ResourceRequest&& request, SecurityCh
         if (RuntimeEnabledFeatures::sharedFeatures().resourceTimingEnabled())
             newRequest.setInitiator(m_options.initiator);
         newRequest.mutableResourceRequest().setAllowCookies(m_options.allowCredentials == AllowStoredCredentials);
+        newRequest.setOrigin(&securityOrigin());
 
         ASSERT(!m_resource);
         // We create an URL here as the request will be moved in requestRawResource
index 02ac342..d566f00 100644 (file)
@@ -121,6 +121,7 @@ CachedResource::CachedResource(CachedResourceRequest&& request, Type type, Sessi
     , m_sessionID(sessionID)
     , m_loadPriority(defaultPriorityForResourceType(type))
     , m_responseTimestamp(std::chrono::system_clock::now())
+    , m_origin(request.releaseOrigin())
     , m_lastDecodedAccessTime(0)
     , m_loadFinishTime(0)
     , m_encodedSize(0)
@@ -148,6 +149,13 @@ CachedResource::CachedResource(CachedResourceRequest&& request, Type type, Sessi
 #ifndef NDEBUG
     cachedResourceLeakCounter.increment();
 #endif
+    // 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()))
+        setCrossOrigin();
 
     if (!m_resourceRequest.url().hasFragmentIdentifier())
         return;
@@ -244,24 +252,6 @@ void CachedResource::addAdditionalRequestHeaders(CachedResourceLoader& loader)
     addAdditionalRequestHeadersToRequest(m_resourceRequest, loader, *this);
 }
 
-void CachedResource::computeOrigin(CachedResourceLoader& loader)
-{
-    if (type() == MainResource)
-        return;
-
-    ASSERT(loader.document());
-    if (m_resourceRequest.hasHTTPOrigin())
-        m_origin = SecurityOrigin::createFromString(m_resourceRequest.httpOrigin());
-    else
-        m_origin = loader.document()->securityOrigin();
-    ASSERT(m_origin);
-
-    if (!(m_resourceRequest.url().protocolIsData() && m_options.sameOriginDataURLFlag == SameOriginDataURLFlag::Set) && !m_origin->canRequest(m_resourceRequest.url()))
-        setCrossOrigin();
-
-    addAdditionalRequestHeaders(loader);
-}
-
 void CachedResource::load(CachedResourceLoader& cachedResourceLoader)
 {
     if (!cachedResourceLoader.frame()) {
@@ -330,7 +320,7 @@ void CachedResource::load(CachedResourceLoader& cachedResourceLoader)
 #endif
     m_resourceRequest.setPriority(loadPriority());
 
-    computeOrigin(cachedResourceLoader);
+    addAdditionalRequestHeaders(cachedResourceLoader);
 
     // 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.
@@ -352,14 +342,12 @@ void CachedResource::load(CachedResourceLoader& cachedResourceLoader)
     m_status = Pending;
 }
 
-void CachedResource::loadFrom(const CachedResource& resource, CachedResourceLoader& cachedResourceLoader)
+void CachedResource::loadFrom(const CachedResource& resource)
 {
     ASSERT(url() == resource.url());
     ASSERT(type() == resource.type());
     ASSERT(resource.status() == Status::Cached);
 
-    computeOrigin(cachedResourceLoader);
-
     if (isCrossOrigin() && m_options.mode == FetchOptions::Mode::Cors) {
         ASSERT(m_origin);
         String errorMessage;
index 45e921f..ba45123 100644 (file)
@@ -211,7 +211,7 @@ public:
     bool isClean() const;
     ResourceResponse::Tainting responseTainting() const { return m_responseTainting; }
 
-    void loadFrom(const CachedResource&, CachedResourceLoader&);
+    void loadFrom(const CachedResource&);
 
     SecurityOrigin* origin() const { return m_origin.get(); }
 
@@ -309,7 +309,6 @@ private:
     std::chrono::microseconds freshnessLifetime(const ResourceResponse&) const;
 
     void addAdditionalRequestHeaders(CachedResourceLoader&);
-    void computeOrigin(CachedResourceLoader&);
     void failBeforeStarting();
 
     HashMap<CachedResourceClient*, std::unique_ptr<Callback>> m_clientsAwaitingCallback;
index a94cbc1..37ca7c0 100644 (file)
@@ -556,7 +556,7 @@ CachedResourceHandle<CachedResource> CachedResourceLoader::updateCachedResourceW
     }
 
     auto resourceHandle = createResource(resource.type(), WTFMove(request), sessionID());
-    resourceHandle->loadFrom(resource, *this);
+    resourceHandle->loadFrom(resource);
     return resourceHandle;
 }
 
@@ -595,6 +595,9 @@ void CachedResourceLoader::prepareFetch(CachedResource::Type type, CachedResourc
 {
     // Implementing step 1 to 7 of https://fetch.spec.whatwg.org/#fetching
 
+    if (!request.origin() && document())
+        request.setOrigin(document()->securityOrigin());
+
     if (!request.resourceRequest().hasHTTPHeader(HTTPHeaderName::Accept))
         request.mutableResourceRequest().setHTTPHeaderField(HTTPHeaderName::Accept, acceptHeaderValueFromType(type));
 
index b86732a..1c3ee6f 100644 (file)
@@ -79,13 +79,16 @@ const AtomicString& CachedResourceRequest::initiatorName() const
 void CachedResourceRequest::setAsPotentiallyCrossOrigin(const String& mode, Document& document)
 {
     ASSERT(m_options.mode == FetchOptions::Mode::NoCors);
+    ASSERT(document.securityOrigin());
+
+    m_origin = document.securityOrigin();
+
     if (mode.isNull())
         return;
     m_options.mode = FetchOptions::Mode::Cors;
     m_options.credentials = equalLettersIgnoringASCIICase(mode, "use-credentials") ? FetchOptions::Credentials::Include : FetchOptions::Credentials::SameOrigin;
     m_options.allowCredentials = equalLettersIgnoringASCIICase(mode, "use-credentials") ? AllowStoredCredentials : DoNotAllowStoredCredentials;
 
-    ASSERT(document.securityOrigin());
     updateRequestForAccessControl(m_resourceRequest, *document.securityOrigin(), m_options.allowCredentials);
 }
 
index a1d80ee..d27b5e3 100644 (file)
@@ -31,6 +31,7 @@
 #include "ResourceLoadPriority.h"
 #include "ResourceLoaderOptions.h"
 #include "ResourceRequest.h"
+#include "SecurityOrigin.h"
 #include <wtf/RefPtr.h>
 #include <wtf/text/AtomicString.h>
 
@@ -62,6 +63,9 @@ public:
     void setCachingPolicy(CachingPolicy policy) { m_options.cachingPolicy = policy; }
 
     void setAsPotentiallyCrossOrigin(const String&, Document&);
+    void setOrigin(RefPtr<SecurityOrigin>&& origin) { ASSERT(!m_origin); m_origin = WTFMove(origin); }
+    RefPtr<SecurityOrigin> releaseOrigin() { return WTFMove(m_origin); }
+    SecurityOrigin* origin() const { return m_origin.get(); }
 
 private:
     ResourceRequest m_resourceRequest;
@@ -72,6 +76,7 @@ private:
     DeferOption m_defer;
     RefPtr<Element> m_initiatorElement;
     AtomicString m_initiatorName;
+    RefPtr<SecurityOrigin> m_origin;
 };
 
 } // namespace WebCore