Source/WebCore: CachedResourceLoader ignores the Range header
authorjaphet@chromium.org <japhet@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Feb 2012 00:47:02 +0000 (00:47 +0000)
committerjaphet@chromium.org <japhet@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Feb 2012 00:47:02 +0000 (00:47 +0000)
in determineRavlidationPolicy(), resulting in incorrect
cache hits.

Reviewed by Adam Barth.

Test: http/tests/xmlhttprequest/range-test.html
Test written by Aaron Colwell (acolwell@chromium.org).

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

LayoutTests: Test Range header compliance for CachedResources.
https://bugs.webkit.org/show_bug.cgi?id=76564

Reviewed by Adam Barth.

* http/tests/xmlhttprequest/range-test-expected.txt: Added.
* http/tests/xmlhttprequest/range-test.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/xmlhttprequest/range-test-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/xmlhttprequest/range-test.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CachedResourceLoader.cpp

index 04cc0c1..74c4c0e 100644 (file)
@@ -1,3 +1,13 @@
+2012-02-22  Nate Chapin  <japhet@chromium.org>
+
+        Test Range header compliance for CachedResources.
+        https://bugs.webkit.org/show_bug.cgi?id=76564
+
+        Reviewed by Adam Barth.
+
+        * http/tests/xmlhttprequest/range-test-expected.txt: Added.
+        * http/tests/xmlhttprequest/range-test.html: Added.
+
 2012-02-22  Adrienne Walker  <enne@google.com>
 
         [chromium] Unreviewed gardening. Generalize svg failures from r108494.
diff --git a/LayoutTests/http/tests/xmlhttprequest/range-test-expected.txt b/LayoutTests/http/tests/xmlhttprequest/range-test-expected.txt
new file mode 100644 (file)
index 0000000..f5743c8
--- /dev/null
@@ -0,0 +1,10 @@
+Test Range support in XMLHttpRequest
+
+getRange(resources/reply.xml, 34, 36, false)
+Length : expected 2 got 2
+getRange(resources/reply.xml, 35, 38, false)
+Length : expected 3 got 3
+getRange(resources/reply.xml, 34, 36, true)
+Length : expected 2 got 2
+getRange(resources/reply.xml, 35, 38, true)
+Length : expected 3 got 3
diff --git a/LayoutTests/http/tests/xmlhttprequest/range-test.html b/LayoutTests/http/tests/xmlhttprequest/range-test.html
new file mode 100644 (file)
index 0000000..c8af0fd
--- /dev/null
@@ -0,0 +1,91 @@
+<html>
+<body>
+<p>Test Range support in XMLHttpRequest</p>
+<script>
+    if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+        layoutTestController.waitUntilDone();
+    }
+
+    var console_messages = document.createElement("ul");
+    document.body.appendChild(console_messages);
+
+    function log(message)
+    {
+        var item = document.createElement("li");
+        item.appendChild(document.createTextNode(message));
+        console_messages.appendChild(item);
+    }
+
+    function getRange(url, start, end, useUniqueUrls, fullResponse, done_callback) {
+        log("getRange(" + url + ", " + start + ", " + end + ", " + useUniqueUrls + ")");
+        var req = new XMLHttpRequest;
+
+        if (useUniqueUrls)
+          url = url + "?q=" + start + "-" + end;
+
+        req.open("GET", url, true);
+        req.setRequestHeader("Range", "bytes=" + start + "-" + (end - 1));
+        req.responseType = 'arraybuffer';
+        req.onreadystatechange = function() {
+            if (req.readyState == 4) {
+                if (req.status != 206)
+                    log("Expected a 206 response, got " + req.status);
+
+                var expected = fullResponse.subarray(start, end);
+                var actual = new Uint8Array(req.response);
+                log("Length : expected " + expected.length + " got " + actual.length);
+
+                if (expected.length == actual.length) {
+                    // Verify that all the data is what we expect.
+                    for (var i = 0; i < expected.length; ++i) {
+                        if (actual[i] != expected[i]) {
+                            log("actual[" + i + "] != expected[" + i + "] (" + actual[i] + " != " + expected[i] + ")");
+                        }
+                    }
+                }
+                done_callback();
+            }
+        };
+        req.send(null);
+    }
+
+    function runRangeTest(useUniqueUrls, done_callback) {
+        // First fetch full resource.
+        var url = "resources/reply.xml";
+        var req = new XMLHttpRequest;
+        req.open("GET", url, true);
+        req.responseType = 'arraybuffer';
+        req.onreadystatechange = function() {
+              if (req.readyState == 4) {
+                  var fullResponse = new Uint8Array(req.response);
+                  var startOffset = fullResponse.length / 2;
+                  var endOffset = startOffset + 2;
+
+                  // Next fetch a subrange in the middle of the resource.
+                  getRange(url, startOffset, endOffset, useUniqueUrls, fullResponse, function() {
+
+                      // Fetch a different range near the middle of the resource.
+                      getRange(url, startOffset + 1, endOffset + 2, useUniqueUrls, fullResponse, function() {
+                          done_callback();
+                      });
+                  });
+              }
+          };
+        req.send(null);
+    }
+
+    // First test range requests using the exact same URL.
+    runRangeTest(false, function() {
+        // Next test w/ a query parameter appended so that each XHR
+        // has a unique URL.
+        runRangeTest(true, function() {
+            if (window.layoutTestController)
+                layoutTestController.notifyDone();
+
+        });
+    });
+
+</script>
+</body>
+</html>
index 1d1535e..a442c2a 100644 (file)
@@ -1,3 +1,17 @@
+2012-02-22  Nate Chapin  <japhet@chromium.org>
+
+        CachedResourceLoader ignores the Range header
+        in determineRavlidationPolicy(), resulting in incorrect
+        cache hits.
+
+        Reviewed by Adam Barth.
+
+        Test: http/tests/xmlhttprequest/range-test.html
+        Test written by Aaron Colwell (acolwell@chromium.org).
+
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::determineRevalidationPolicy):
+
 2012-02-22  Antti Koivisto  <antti@apple.com>
 
         REGRESSION (r104060): Web font is not loaded if specified by link element dynamically inserted
index 9b40bd4..e8fc38a 100644 (file)
@@ -531,6 +531,11 @@ CachedResourceLoader::RevalidationPolicy CachedResourceLoader::determineRevalida
     // of things about how revalidation works that manual headers violate, so punt to Reload instead.
     if (request.isConditional())
         return Reload;
+
+    // Re-using resources in the case of a Range header is very simple if the headers are identical and
+    // much tougher if they aren't.
+    if (existingResource->resourceRequest().httpHeaderField("Range") != request.httpHeaderField("Range"))
+        return Reload;
     
     // Don't reload resources while pasting.
     if (m_allowStaleResources)