Don't reuse memory cache entries with different charset
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Jan 2016 17:31:02 +0000 (17:31 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Jan 2016 17:31:02 +0000 (17:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=110031
Source/WebCore:

rdar://problem/13666418

Reviewed by Andreas Kling.

Test: fast/loader/cache-encoding.html

* loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::requestResource):
(WebCore::logResourceRevalidationDecision):
(WebCore::CachedResourceLoader::determineRevalidationPolicy):

    Pass full CachedResourceRequest to the function.
    If charset differs don't reuse the cache entry.

* loader/cache/CachedResourceLoader.h:

LayoutTests:

Reviewed by Andreas Kling.

* fast/loader/cache-encoding-expected.txt: Added.
* fast/loader/cache-encoding.html: Added.
* fast/loader/resources/success.js: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/loader/cache-encoding-expected.txt [new file with mode: 0644]
LayoutTests/fast/loader/cache-encoding.html [new file with mode: 0644]
LayoutTests/fast/loader/resources/success.js [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CachedResourceLoader.cpp
Source/WebCore/loader/cache/CachedResourceLoader.h

index 6b7e69a..806e9ad 100644 (file)
@@ -1,3 +1,14 @@
+2016-01-12  Antti Koivisto  <antti@apple.com>
+
+        Don't reuse memory cache entries with different charset
+        https://bugs.webkit.org/show_bug.cgi?id=110031
+
+        Reviewed by Andreas Kling.
+
+        * fast/loader/cache-encoding-expected.txt: Added.
+        * fast/loader/cache-encoding.html: Added.
+        * fast/loader/resources/success.js: Added.
+
 2016-01-12  Ryan Haddad  <ryanhaddad@apple.com>
 
         Marking inspector/model/scope-chain-node.html as flaky on Mac debug
diff --git a/LayoutTests/fast/loader/cache-encoding-expected.txt b/LayoutTests/fast/loader/cache-encoding-expected.txt
new file mode 100644 (file)
index 0000000..785d97d
--- /dev/null
@@ -0,0 +1,11 @@
+CONSOLE MESSAGE: line 1: SyntaxError: Invalid character '\u8307'
+First load a script with a wrong charset then again with the right one. Second attempt should work and 'scriptSuccess' should be set to true. 'successfullyParsed' will be undefined.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS scriptSuccess is true
+FAIL successfullyParsed should be true (of type boolean). Was undefined (of type undefined).
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/loader/cache-encoding.html b/LayoutTests/fast/loader/cache-encoding.html
new file mode 100644 (file)
index 0000000..38af9b6
--- /dev/null
@@ -0,0 +1,28 @@
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+jsTestIsAsync = true;
+
+description("First load a script with a wrong charset then again with the right one. Second attempt should work and 'scriptSuccess' should be set to true. 'successfullyParsed' will be undefined.");
+
+function appendScriptWithCharset(charset, onload)
+{
+    var script = document.createElement("script");
+    script.src = "resources/success.js";
+    script.setAttribute("charset", charset);
+    script.onload = onload;
+    script.onerror = onload;
+    document.body.appendChild(script);
+}
+
+function test()
+{
+    appendScriptWithCharset("utf-16", () => {
+        appendScriptWithCharset("utf-8", () => {
+            shouldBeTrue("scriptSuccess");
+            finishJSTest();
+        });
+    });
+}
+</script>
+<body onload="test()">
+<script src="../../resources/js-test-post.js"></script>
diff --git a/LayoutTests/fast/loader/resources/success.js b/LayoutTests/fast/loader/resources/success.js
new file mode 100644 (file)
index 0000000..3032cdf
--- /dev/null
@@ -0,0 +1 @@
+scriptSuccess = true;
index d71563f..6fc4610 100644 (file)
@@ -1,3 +1,23 @@
+2016-01-12  Antti Koivisto  <antti@apple.com>
+
+        Don't reuse memory cache entries with different charset
+        https://bugs.webkit.org/show_bug.cgi?id=110031
+        rdar://problem/13666418
+
+        Reviewed by Andreas Kling.
+
+        Test: fast/loader/cache-encoding.html
+
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::requestResource):
+        (WebCore::logResourceRevalidationDecision):
+        (WebCore::CachedResourceLoader::determineRevalidationPolicy):
+
+            Pass full CachedResourceRequest to the function.
+            If charset differs don't reuse the cache entry.
+
+        * loader/cache/CachedResourceLoader.h:
+
 2016-01-12  Gyuyoung Kim  <gyuyoung.kim@webkit.org>
 
         Use a pointer instead of PassRefPtr in AbstractView argument of UIEvent class
index 36bab32..b9faede 100644 (file)
@@ -574,7 +574,7 @@ CachedResourceHandle<CachedResource> CachedResourceLoader::requestResource(Cache
 
     logMemoryCacheResourceRequest(frame(), resource ? DiagnosticLoggingKeys::inMemoryCacheKey() : DiagnosticLoggingKeys::notInMemoryCacheKey());
 
-    const RevalidationPolicy policy = determineRevalidationPolicy(type, request.mutableResourceRequest(), request.forPreload(), resource.get(), request.defer());
+    const RevalidationPolicy policy = determineRevalidationPolicy(type, request, resource.get());
     switch (policy) {
     case Reload:
         memoryCache.remove(*resource);
@@ -713,13 +713,15 @@ static void logResourceRevalidationDecision(CachedResource::RevalidationDecision
     }
 }
 
-CachedResourceLoader::RevalidationPolicy CachedResourceLoader::determineRevalidationPolicy(CachedResource::Type type, ResourceRequest& request, bool forPreload, CachedResource* existingResource, CachedResourceRequest::DeferOption defer) const
+CachedResourceLoader::RevalidationPolicy CachedResourceLoader::determineRevalidationPolicy(CachedResource::Type type, CachedResourceRequest& cachedResourceRequest, CachedResource* existingResource) const
 {
+    auto& request = cachedResourceRequest.resourceRequest();
+
     if (!existingResource)
         return Load;
 
     // We already have a preload going for this URL.
-    if (forPreload && existingResource->isPreloaded())
+    if (cachedResourceRequest.forPreload() && existingResource->isPreloaded())
         return Use;
 
     // If the same URL has been loaded as a different type, we need to reload.
@@ -729,6 +731,9 @@ CachedResourceLoader::RevalidationPolicy CachedResourceLoader::determineRevalida
         return Reload;
     }
 
+    if (existingResource->encoding() != TextEncoding(cachedResourceRequest.charset()))
+        return Reload;
+
     // FIXME: We should use the same cache policy for all resource types. The raw resource policy is overly strict
     //        while the normal subresource policy is too loose.
     if (existingResource->isMainOrRawResource()) {
@@ -743,7 +748,7 @@ CachedResourceLoader::RevalidationPolicy CachedResourceLoader::determineRevalida
 
     // Do not load from cache if images are not enabled. The load for this image will be blocked
     // in CachedImage::load.
-    if (CachedResourceRequest::DeferredByClient == defer)
+    if (cachedResourceRequest.defer() == CachedResourceRequest::DeferredByClient)
         return Reload;
     
     // Don't reload resources while pasting.
index 5c10448..d1bc44e 100644 (file)
@@ -148,7 +148,7 @@ private:
     void requestPreload(CachedResource::Type, CachedResourceRequest&, const String& charset);
 
     enum RevalidationPolicy { Use, Revalidate, Reload, Load };
-    RevalidationPolicy determineRevalidationPolicy(CachedResource::Type, ResourceRequest&, bool forPreload, CachedResource* existingResource, CachedResourceRequest::DeferOption) const;
+    RevalidationPolicy determineRevalidationPolicy(CachedResource::Type, CachedResourceRequest&, CachedResource* existingResource) const;
     
     bool shouldContinueAfterNotifyingLoadedFromMemoryCache(const CachedResourceRequest&, CachedResource*);
     bool checkInsecureContent(CachedResource::Type, const URL&) const;