Do not reuse cache entries with conditional headers
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 May 2016 18:05:46 +0000 (18:05 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 May 2016 18:05:46 +0000 (18:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157205
rdar://problem/25856933

Reviewed by Chris Dumez.

Source/WebCore:

Test: http/tests/xmlhttprequest/if-modified-since-0.html

* loader/cache/CachedRawResource.cpp:
(WebCore::CachedRawResource::canReuse):
CachedResourceLoader::determineRevalidationPolicy asserts that the request is not conditional,
which means that it does not have any headers like If-Modified-Since.  They are usually different,
because we put the timestamp in the If-Modified-Since header, so it fails the canReuse test because
time has passed since the last If-Modified-Since header was sent.  When a user sets the If-Modified-Since
manually to something that is constant, we reuse cache entries when we should not.
* platform/network/mac/WebCoreResourceHandleAsDelegate.mm:
(-[WebCoreResourceHandleAsDelegate connection:didReceiveResponse:]):
Set the source so we can use it in Internals.

LayoutTests:

* http/tests/xmlhttprequest/if-modified-since-0-expected.txt: Added.
* http/tests/xmlhttprequest/if-modified-since-0.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/xmlhttprequest/if-modified-since-0-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/xmlhttprequest/if-modified-since-0.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CachedRawResource.cpp
Source/WebCore/platform/network/mac/WebCoreResourceHandleAsDelegate.mm

index 321c2c4..aef7bdd 100644 (file)
@@ -1,3 +1,14 @@
+2016-04-29  Alex Christensen  <achristensen@webkit.org>
+
+        Do not reuse cache entries with conditional headers
+        https://bugs.webkit.org/show_bug.cgi?id=157205
+        rdar://problem/25856933
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/xmlhttprequest/if-modified-since-0-expected.txt: Added.
+        * http/tests/xmlhttprequest/if-modified-since-0.html: Added.
+
 2016-05-01  Skachkov Oleksandr  <gskachkov@gmail.com>
 
         Class contructor and methods shouldn't have "arguments" and "caller"
diff --git a/LayoutTests/http/tests/xmlhttprequest/if-modified-since-0-expected.txt b/LayoutTests/http/tests/xmlhttprequest/if-modified-since-0-expected.txt
new file mode 100644 (file)
index 0000000..1d4084b
--- /dev/null
@@ -0,0 +1,3 @@
+ALERT: PASS
+ALERT: PASS
+
diff --git a/LayoutTests/http/tests/xmlhttprequest/if-modified-since-0.html b/LayoutTests/http/tests/xmlhttprequest/if-modified-since-0.html
new file mode 100644 (file)
index 0000000..a7fea21
--- /dev/null
@@ -0,0 +1,29 @@
+<script>
+
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+var req1 = new XMLHttpRequest();
+req1.onreadystatechange = function () {
+    if (this.readyState == 4) {
+        alert(internals.xhrResponseSource(req1) == "Network" ? "PASS" : "FAIL");
+        var req2 = new XMLHttpRequest();
+        req2.onreadystatechange = function () {
+            if (this.readyState == 4) {
+                alert(internals.xhrResponseSource(req2) == "Network" ? "PASS" : "FAIL");
+                if (window.testRunner)
+                    testRunner.notifyDone();
+            }
+        }
+        req2.open("GET", "resources/get.txt");
+        req2.setRequestHeader("If-Modified-Since", "0");
+        req2.send(null);
+    }
+};
+req1.open("GET", "resources/get.txt");
+req1.setRequestHeader("If-Modified-Since", "0");
+req1.send(null);
+
+</script>
index 2e28c49..1aebef9 100644 (file)
@@ -1,3 +1,24 @@
+2016-04-29  Alex Christensen  <achristensen@webkit.org>
+
+        Do not reuse cache entries with conditional headers
+        https://bugs.webkit.org/show_bug.cgi?id=157205
+        rdar://problem/25856933
+
+        Reviewed by Chris Dumez.
+
+        Test: http/tests/xmlhttprequest/if-modified-since-0.html
+
+        * loader/cache/CachedRawResource.cpp:
+        (WebCore::CachedRawResource::canReuse):
+        CachedResourceLoader::determineRevalidationPolicy asserts that the request is not conditional,
+        which means that it does not have any headers like If-Modified-Since.  They are usually different,
+        because we put the timestamp in the If-Modified-Since header, so it fails the canReuse test because
+        time has passed since the last If-Modified-Since header was sent.  When a user sets the If-Modified-Since
+        manually to something that is constant, we reuse cache entries when we should not.
+        * platform/network/mac/WebCoreResourceHandleAsDelegate.mm:
+        (-[WebCoreResourceHandleAsDelegate connection:didReceiveResponse:]):
+        Set the source so we can use it in Internals.
+
 2016-05-02  Yoav Weiss  <yoav@yoav.ws>
 
         Speculatively fix the cmake build
index 709d1ee..821a888 100644 (file)
@@ -254,6 +254,9 @@ bool CachedRawResource::canReuse(const ResourceRequest& newRequest) const
     if (m_resourceRequest.allowCookies() != newRequest.allowCookies())
         return false;
 
+    if (newRequest.isConditional())
+        return false;
+
     // Ensure most headers match the existing headers before continuing.
     // Note that the list of ignored headers includes some headers explicitly related to caching.
     // A more detailed check of caching policy will be performed later, this is simply a list of
index 237ea9b..c80048f 100644 (file)
@@ -142,10 +142,6 @@ using namespace WebCore;
 
 - (void)connection:(NSURLConnection *)connection didReceiveResponse:(NSURLResponse *)r
 {
-#if !PLATFORM(IOS)
-    UNUSED_PARAM(connection);
-#endif
-
     LOG(Network, "Handle %p delegate connection:%p didReceiveResponse:%p (HTTP status %d, reported MIMEType '%s')", m_handle, connection, r, [r respondsToSelector:@selector(statusCode)] ? [(id)r statusCode] : 0, [[r MIMEType] UTF8String]);
 
     if (!m_handle || !m_handle->client())
@@ -170,6 +166,7 @@ using namespace WebCore;
 #endif
     
     ResourceResponse resourceResponse(r);
+    resourceResponse.setSource(ResourceResponse::Source::Network);
 #if ENABLE(WEB_TIMING)
     ResourceHandle::getConnectionTimingData(connection, resourceResponse.resourceLoadTiming());
 #else