Don't reuse memory cache entries with different charset

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. Canonical link: https://commits.webkit.org/171102@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@194898 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jan 12, 2016 · e5ede6ff56d16594a183bc216904bedd255f1d63 · e5ede6f
1 parent dfd01b9
commit e5ede6ff56d16594a183bc216904bedd255f1d63
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -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
@@ -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
@@ -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
@@ -0,0 +1 @@
+scriptSuccess = true;
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -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

diff --git a/Source/WebCore/loader/cache/CachedResourceLoader.cpp b/Source/WebCore/loader/cache/CachedResourceLoader.cpp
@@ -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.

diff --git a/Source/WebCore/loader/cache/CachedResourceLoader.h b/Source/WebCore/loader/cache/CachedResourceLoader.h
@@ -148,7 +148,7 @@ friend class ResourceCacheValidationSuppressor;
     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;