Changeset 202985 in webkit

Timestamp:

Jul 8, 2016, 10:11:20 AM (9 years ago)

Author:

Antti Koivisto

Message:

Regression(r201805): Crash with <use> resource that has Vary header
​https://bugs.webkit.org/show_bug.cgi?id=159560
<rdar://problem/27034208>

Reviewed by Chris Dumez.

Source/WebCore:

In some situations (SVG <use> element for example) we may try to load resources from frameless documents.
Such loads always fail. The new vary header verification code path tried to access the frame earlier without
null check.

Test: http/tests/cache/vary-frameless-document.html

• loader/cache/CachedResource.cpp:

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

Null check frame.
Also move the resource type check here so all callers get the same behavior.

(WebCore::CachedResource::addAdditionalRequestHeaders):
(WebCore::CachedResource::load):
(WebCore::CachedResource::varyHeaderValuesMatch):

LayoutTests:

• http/tests/cache/resources/svg-defs-vary.php: Added.
• http/tests/cache/vary-frameless-document-expected.txt: Added.
• http/tests/cache/vary-frameless-document.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/http/tests/cache/resources/svg-defs-vary.php (added)
• LayoutTests/http/tests/cache/vary-frameless-document-expected.txt (added)
• LayoutTests/http/tests/cache/vary-frameless-document.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/cache/CachedResource.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-07-08  Antti Koivisto  <antti@apple.com>
+
+        Regression(r201805): Crash with <use> resource that has Vary header
+        https://bugs.webkit.org/show_bug.cgi?id=159560
+        <rdar://problem/27034208>
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/cache/resources/svg-defs-vary.php: Added.
+        * http/tests/cache/vary-frameless-document-expected.txt: Added.
+        * http/tests/cache/vary-frameless-document.html: Added.
+
 2016-07-08  Commit Queue  <commit-queue@webkit.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-07-08  Antti Koivisto  <antti@apple.com>
+
+        Regression(r201805): Crash with <use> resource that has Vary header
+        https://bugs.webkit.org/show_bug.cgi?id=159560
+        <rdar://problem/27034208>
+
+        Reviewed by Chris Dumez.
+
+        In some situations (SVG <use> element for example) we may try to load resources from frameless documents.
+        Such loads always fail. The new vary header verification code path tried to access the frame earlier without
+        null check.
+
+        Test: http/tests/cache/vary-frameless-document.html
+
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::failBeforeStarting):
+        (WebCore::addAdditionalRequestHeadersToRequest):
+
+            Null check frame.
+            Also move the resource type check here so all callers get the same behavior.
+
+        (WebCore::CachedResource::addAdditionalRequestHeaders):
+        (WebCore::CachedResource::load):
+        (WebCore::CachedResource::varyHeaderValuesMatch):
+
 2016-07-08  Brady Eidson  <beidson@apple.com>
 

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

@@ -184 +184 @@
 }
 
-static void addAdditionalRequestHeadersToRequest(ResourceRequest& request, const CachedResourceLoader& cachedResourceLoader)
-{
+static void addAdditionalRequestHeadersToRequest(ResourceRequest& request, const CachedResourceLoader& cachedResourceLoader, CachedResource::Type type)
+{
+    if (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;
@@ -214 +220 @@
 void CachedResource::addAdditionalRequestHeaders(CachedResourceLoader& cachedResourceLoader)
 {
-    addAdditionalRequestHeadersToRequest(m_resourceRequest, cachedResourceLoader);
+    addAdditionalRequestHeadersToRequest(m_resourceRequest, cachedResourceLoader, type());
 }
 
@@ -276 +282 @@
     m_resourceRequest.setPriority(loadPriority());
 
-    if (type() != MainResource)
-        addAdditionalRequestHeaders(cachedResourceLoader);
+    addAdditionalRequestHeaders(cachedResourceLoader);
 
     // FIXME: It's unfortunate that the cache layer and below get to know anything about fragment identifiers.
@@ -781 +786 @@
 
     ResourceRequest requestWithFullHeaders(request);
-    addAdditionalRequestHeadersToRequest(requestWithFullHeaders, cachedResourceLoader);
+    addAdditionalRequestHeadersToRequest(requestWithFullHeaders, cachedResourceLoader, type());
 
     return verifyVaryingRequestHeaders(m_varyingHeaderValues, requestWithFullHeaders, m_sessionID);