[AppCache] Ignore fallback entries whose namespace is not prefixed with manifest...
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 7 Jul 2017 22:02:46 +0000 (22:02 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 7 Jul 2017 22:02:46 +0000 (22:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174273
<rdar://problem/33011682>

Reviewed by Brent Fulgham.

Source/WebCore:

As per <https://html.spec.whatwg.org/multipage/offline.html#parsing-cache-manifests> (07/06/2017)
we should ignore fallback entires whose fallback namespace URL is not prefixed with
the manifest path. For now we only apply this policy when the manifest is served with
a non-standard Content-Type to minimize web compatibility risk.

Test: http/tests/appcache/fallback-namespace-outside-manifest-path.html

* loader/appcache/ApplicationCacheGroup.cpp:
(WebCore::ApplicationCacheGroup::didFinishLoadingManifest): Pass the MIME type of the manifest.
* loader/appcache/ManifestParser.cpp:
(WebCore::manifestPath): Computes the manifest path from a manifest URL.
(WebCore::parseManifest): Modified to take the MIME type of the manifest. If the MIME type is
non-standard (i.e. not text/cached-manifest) then skip fallback entries whose namespace is not
prefixed with the manifest path. Otherwise, process fallback entries as we do now. Also cleaned
up the code a bit while I was here, including renaming a local variable to be more descriptive
and using a const character array for the manifest signature to avoid the need to document the
length of the manifest signature in a comment.
* loader/appcache/ManifestParser.h:

LayoutTests:

* http/tests/appcache/fallback-namespace-outside-manifest-path-expected.txt: Added.
* http/tests/appcache/fallback-namespace-outside-manifest-path.html: Added.
* http/tests/appcache/resources/fallback-namespace-outside-manifest-path.txt: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/appcache/fallback-namespace-outside-manifest-path-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/appcache/fallback-namespace-outside-manifest-path.html [new file with mode: 0644]
LayoutTests/http/tests/appcache/resources/fallback-namespace-outside-manifest-path.txt [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp
Source/WebCore/loader/appcache/ManifestParser.cpp
Source/WebCore/loader/appcache/ManifestParser.h

index e46fa1a..57d2d8b 100644 (file)
@@ -1,3 +1,15 @@
+2017-07-07  Daniel Bates  <dabates@apple.com>
+
+        [AppCache] Ignore fallback entries whose namespace is not prefixed with manifest path
+        https://bugs.webkit.org/show_bug.cgi?id=174273
+        <rdar://problem/33011682>
+
+        Reviewed by Brent Fulgham.
+
+        * http/tests/appcache/fallback-namespace-outside-manifest-path-expected.txt: Added.
+        * http/tests/appcache/fallback-namespace-outside-manifest-path.html: Added.
+        * http/tests/appcache/resources/fallback-namespace-outside-manifest-path.txt: Added.
+
 2017-07-07  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Show all elements currently using a given CSS Canvas
diff --git a/LayoutTests/http/tests/appcache/fallback-namespace-outside-manifest-path-expected.txt b/LayoutTests/http/tests/appcache/fallback-namespace-outside-manifest-path-expected.txt
new file mode 100644 (file)
index 0000000..9805c6b
--- /dev/null
@@ -0,0 +1,7 @@
+Tests that we do not load the fallback entry when the manifest does not have Content-Type text/cached-manifest and the fallback namespace is outside the manifest path.
+
+Fallback namespace under the manifest path:
+PASS did load fallback entry with fallback namespace under manifest path.
+Fallback namespace outside of manifest path:
+PASS did not load fallback entry with fallback namespace outside manifest path.
+
diff --git a/LayoutTests/http/tests/appcache/fallback-namespace-outside-manifest-path.html b/LayoutTests/http/tests/appcache/fallback-namespace-outside-manifest-path.html
new file mode 100644 (file)
index 0000000..b1f5d8b
--- /dev/null
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<!-- Explicitly use manifest resource that does not end in .manifest so that its Content-Type is not text/cached-manifest. -->
+<html manifest="resources/fallback-namespace-outside-manifest-path.txt">
+<head>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+</script>
+</head>
+<body>
+<p>Tests that we do not load the fallback entry when the manifest does not have Content-Type text/cached-manifest and the fallback namespace is outside the manifest path.</p>
+<pre id="console"></pre>
+<script>
+window.applicationCache.onnoupdate = runTests;
+window.applicationCache.oncached = runTests;
+
+window.applicationCache.onupdateready = () => logMessage("FAIL received unexpected updateready event");
+window.applicationCache.onerror = () => logMessage("FAIL received unexpected error event");
+
+function loadURL(url)
+{
+    var xhr = new XMLHttpRequest;
+    xhr.open("GET", url, false /* synchronous */);
+    xhr.send("");
+    return xhr.responseText;
+}
+
+function logMessage(message)
+{
+    document.getElementById("console").appendChild(document.createTextNode(message + "\n"));
+}
+
+function runTests()
+{
+    logMessage("Fallback namespace under the manifest path:");
+    try {
+        var responseText = loadURL("resources/non-existent-file-under-manifest-path.html");
+        console.assert(responseText === "Hello, World!");
+        logMessage("PASS did load fallback entry with fallback namespace under manifest path.");
+    } catch (e) {
+        logMessage("FAIL did load fallback entry with fallback namespace under manifest path.");
+    }
+
+    logMessage("Fallback namespace outside of manifest path:");
+    try {
+        loadURL("/non-existent-file-outside-manifest-path.html");
+        logMessage("FAIL did load fallback entry with fallback namespace outside manifest path.");
+    } catch (e) {
+        logMessage("PASS did not load fallback entry with fallback namespace outside manifest path.");
+    }
+
+    if (window.testRunner)
+        testRunner.notifyDone();
+}
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/appcache/resources/fallback-namespace-outside-manifest-path.txt b/LayoutTests/http/tests/appcache/resources/fallback-namespace-outside-manifest-path.txt
new file mode 100644 (file)
index 0000000..270d815
--- /dev/null
@@ -0,0 +1,6 @@
+CACHE MANIFEST
+# Note that we use a filename that ends in .txt instead of .manifest so that this manifest
+# is served with a Content-Type that is not text/cached-manifest.
+FALLBACK:
+/non-existent-file-outside-manifest-path.html simple.txt
+non-existent-file-under-manifest-path.html simple.txt
index 14a4c66..9473ab4 100644 (file)
@@ -1,3 +1,30 @@
+2017-07-07  Daniel Bates  <dabates@apple.com>
+
+        [AppCache] Ignore fallback entries whose namespace is not prefixed with manifest path
+        https://bugs.webkit.org/show_bug.cgi?id=174273
+        <rdar://problem/33011682>
+
+        Reviewed by Brent Fulgham.
+
+        As per <https://html.spec.whatwg.org/multipage/offline.html#parsing-cache-manifests> (07/06/2017)
+        we should ignore fallback entires whose fallback namespace URL is not prefixed with
+        the manifest path. For now we only apply this policy when the manifest is served with
+        a non-standard Content-Type to minimize web compatibility risk.
+
+        Test: http/tests/appcache/fallback-namespace-outside-manifest-path.html
+
+        * loader/appcache/ApplicationCacheGroup.cpp:
+        (WebCore::ApplicationCacheGroup::didFinishLoadingManifest): Pass the MIME type of the manifest.
+        * loader/appcache/ManifestParser.cpp:
+        (WebCore::manifestPath): Computes the manifest path from a manifest URL.
+        (WebCore::parseManifest): Modified to take the MIME type of the manifest. If the MIME type is
+        non-standard (i.e. not text/cached-manifest) then skip fallback entries whose namespace is not
+        prefixed with the manifest path. Otherwise, process fallback entries as we do now. Also cleaned
+        up the code a bit while I was here, including renaming a local variable to be more descriptive
+        and using a const character array for the manifest signature to avoid the need to document the
+        length of the manifest signature in a comment.
+        * loader/appcache/ManifestParser.h:
+
 2017-07-07  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [iOS DnD] For cross-app drags, 'drop' event handlers are never invoked if dataTransfer.dropEffect is not set while dragging
index ea5cddb..85b1ea2 100644 (file)
@@ -713,7 +713,7 @@ void ApplicationCacheGroup::didFinishLoadingManifest()
     }
     
     Manifest manifest;
-    if (!parseManifest(m_manifestURL, m_manifestResource->data().data(), m_manifestResource->data().size(), manifest)) {
+    if (!parseManifest(m_manifestURL, m_manifestResource->response().mimeType(), m_manifestResource->data().data(), m_manifestResource->data().size(), manifest)) {
         // At the time of this writing, lack of "CACHE MANIFEST" signature is the only reason for parseManifest to fail.
         m_frame->document()->addConsoleMessage(MessageSource::AppCache, MessageLevel::Error, ASCIILiteral("Application Cache manifest could not be parsed. Does it start with CACHE MANIFEST?"));
         cacheUpdateFailed();
index 5c15135..31c2e79 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008 Apple Inc. All Rights Reserved.
+ * Copyright (C) 2008-2017 Apple Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 namespace WebCore {
 
 enum Mode { Explicit, Fallback, OnlineWhitelist, Unknown };
-    
-bool parseManifest(const URL& manifestURL, const char* data, int length, Manifest& manifest)
+
+static String manifestPath(const URL& manifestURL)
+{
+    String manifestPath = manifestURL.path();
+    ASSERT(manifestPath[0] == '/');
+    manifestPath = manifestPath.substring(0, manifestPath.reverseFind('/') + 1);
+    ASSERT(manifestPath[0] == manifestPath[manifestPath.length() - 1]);
+    return manifestPath;
+}
+
+bool parseManifest(const URL& manifestURL, const String& manifestMIMEType, const char* data, int length, Manifest& manifest)
 {
     ASSERT(manifest.explicitURLs.isEmpty());
     ASSERT(manifest.onlineWhitelistedURLs.isEmpty());
     ASSERT(manifest.fallbackURLs.isEmpty());
     manifest.allowAllNetworkRequests = false;
 
+    String manifestPath = WebCore::manifestPath(manifestURL);
+
+    const char cacheManifestMIMEType[] = "text/cache-manifest";
+    bool allowFallbackNamespaceOutsideManfestPath = equalLettersIgnoringASCIICase(manifestMIMEType, cacheManifestMIMEType);
+
     Mode mode = Explicit;
 
-    String s = TextResourceDecoder::create("text/cache-manifest", "UTF-8")->decodeAndFlush(data, length);
+    String manifestString = TextResourceDecoder::create(ASCIILiteral(cacheManifestMIMEType), "UTF-8")->decodeAndFlush(data, length);
     
     // Look for the magic signature: "^\xFEFF?CACHE MANIFEST[ \t]?" (the BOM is removed by TextResourceDecoder).
     // Example: "CACHE MANIFEST #comment" is a valid signature.
     // Example: "CACHE MANIFEST;V2" is not.
-    if (!s.startsWith("CACHE MANIFEST"))
+    const char manifestSignature[] = "CACHE MANIFEST";
+    if (!manifestString.startsWith(manifestSignature))
         return false;
     
-    StringView manifestAfterSignature = StringView(s).substring(14); // "CACHE MANIFEST" is 14 characters.
+    StringView manifestAfterSignature = StringView(manifestString).substring(sizeof(manifestSignature) - 1);
     auto upconvertedCharacters = manifestAfterSignature.upconvertedCharacters();
     const UChar* p = upconvertedCharacters;
     const UChar* end = p + manifestAfterSignature.length();
@@ -153,7 +168,13 @@ bool parseManifest(const URL& manifestURL, const char* data, int length, Manifes
 
             if (!protocolHostAndPortAreEqual(manifestURL, namespaceURL))
                 continue;
-                                   
+
+            // Although <https://html.spec.whatwg.org/multipage/offline.html#parsing-cache-manifests> (07/06/2017) saids
+            // that we should always prefix match the manifest path we only do so if the manifest was served with a non-
+            // standard HTTP Content-Type header for web compatibility.
+            if (!allowFallbackNamespaceOutsideManfestPath && !namespaceURL.path().startsWith(manifestPath))
+                continue;
+
             // Skip whitespace separating fallback namespace from URL.
             while (p < lineEnd && (*p == '\t' || *p == ' '))
                 p++;
index b28249e..e781089 100644 (file)
@@ -39,6 +39,6 @@ struct Manifest {
     bool allowAllNetworkRequests; // Wildcard found in NETWORK section.
 };
 
-bool parseManifest(const URL& manifestURL, const char* data, int length, Manifest&);
+bool parseManifest(const URL& manifestURL, const String& manifestMIMEType, const char* data, int length, Manifest&);
 
 } // namespace WebCore