Regression(r201805): Crash with <use> resource that has Vary header
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Jul 2016 17:11:20 +0000 (17:11 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Jul 2016 17:11:20 +0000 (17:11 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/cache/resources/svg-defs-vary.php [new file with mode: 0644]
LayoutTests/http/tests/cache/vary-frameless-document-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/cache/vary-frameless-document.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CachedResource.cpp

index 7196410..477dd63 100644 (file)
@@ -1,3 +1,15 @@
+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>
 
         Unreviewed, rolling out r202945.
diff --git a/LayoutTests/http/tests/cache/resources/svg-defs-vary.php b/LayoutTests/http/tests/cache/resources/svg-defs-vary.php
new file mode 100644 (file)
index 0000000..51631e6
--- /dev/null
@@ -0,0 +1,12 @@
+<?php
+header("Content-Type: image/svg+xml");
+header("Vary: Cookie");
+header("Cache-Control: max-age=31536000");
+?>
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+<defs>
+<g id="circle">
+<circle style="fill:green" r="10"/>
+</g>
+</defs>
+</svg>
diff --git a/LayoutTests/http/tests/cache/vary-frameless-document-expected.txt b/LayoutTests/http/tests/cache/vary-frameless-document-expected.txt
new file mode 100644 (file)
index 0000000..23b2b46
--- /dev/null
@@ -0,0 +1 @@
+This test passes if it doesn't crash. 
diff --git a/LayoutTests/http/tests/cache/vary-frameless-document.html b/LayoutTests/http/tests/cache/vary-frameless-document.html
new file mode 100644 (file)
index 0000000..d331c3f
--- /dev/null
@@ -0,0 +1,19 @@
+<head>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+var svgDocumentString = '<svg><use x="50" y="10" xlink:href="resources/svg-defs-vary.php#circle" /></svg>';
+function makeDocument() {
+    var doc = document.implementation.createHTMLDocument('');
+    doc.open();
+    doc.write(svgDocumentString);
+    doc.close();
+}
+</script>
+</head>
+<body onload="makeDocument()">
+This test passes if it doesn't crash.
+<script>
+document.write(svgDocumentString);
+</script>
index e4f12d8..42ea218 100644 (file)
@@ -1,3 +1,28 @@
+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>
 
         Clearing LocalStorage doesn't also delete -wal and -shm files.
index 8df7d5e..ddaa491 100644 (file)
@@ -183,12 +183,18 @@ void CachedResource::failBeforeStarting()
     error(CachedResource::LoadError);
 }
 
-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;
     String outgoingOrigin;
@@ -213,7 +219,7 @@ static void addAdditionalRequestHeadersToRequest(ResourceRequest& request, const
 
 void CachedResource::addAdditionalRequestHeaders(CachedResourceLoader& cachedResourceLoader)
 {
-    addAdditionalRequestHeadersToRequest(m_resourceRequest, cachedResourceLoader);
+    addAdditionalRequestHeadersToRequest(m_resourceRequest, cachedResourceLoader, type());
 }
 
 void CachedResource::load(CachedResourceLoader& cachedResourceLoader, const ResourceLoaderOptions& options)
@@ -275,8 +281,7 @@ void CachedResource::load(CachedResourceLoader& cachedResourceLoader, const Reso
 #endif
     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.
     // We should look into removing the expectation of that knowledge from the platform network stacks.
@@ -780,7 +785,7 @@ bool CachedResource::varyHeaderValuesMatch(const ResourceRequest& request, const
         return true;
 
     ResourceRequest requestWithFullHeaders(request);
-    addAdditionalRequestHeadersToRequest(requestWithFullHeaders, cachedResourceLoader);
+    addAdditionalRequestHeadersToRequest(requestWithFullHeaders, cachedResourceLoader, type());
 
     return verifyVaryingRequestHeaders(m_varyingHeaderValues, requestWithFullHeaders, m_sessionID);
 }