Http Cache: More unit tests for byte range support.

BUG=b/2071330
TEST=unittests

Review URL: http://codereview.chromium.org/267101

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@29035 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 209 insertions, 51 deletions

diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 67d6de8..9df19c2 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -402,6 +402,10 @@ class MockHttpCache {
   MockHttpCache() : http_cache_(new MockNetworkLayer(), new MockDiskCache()) {
   }
 
+  explicit MockHttpCache(disk_cache::Backend* disk_cache)
+      : http_cache_(new MockNetworkLayer(), disk_cache) {
+  }
+
   net::HttpCache* http_cache() { return &http_cache_; }
 
   MockNetworkLayer* network_layer() {
@@ -2434,6 +2438,142 @@ TEST(HttpCache, RangeGET_Cancel2) {
   RemoveMockTransaction(&kRangeGET_TransactionOK);
 }
 
+// Tests that an invalid range response results in no cached entry.
+TEST(HttpCache, RangeGET_InvalidResponse1) {
+  MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
+  std::string headers;
+
+  MockTransaction transaction(kRangeGET_TransactionOK);
+  transaction.handler = NULL;
+  transaction.response_headers = "Content-Range: bytes 40-49/45\n"
+                                 "Content-Length: 10\n";
+  AddMockTransaction(&transaction);
+  RunTransactionTestWithResponse(cache.http_cache(), transaction, &headers);
+
+  std::string expected(transaction.status);
+  expected.append("\n");
+  expected.append(transaction.response_headers);
+  EXPECT_EQ(expected, headers);
+
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  // Verify that we don't have a cached entry.
+  disk_cache::Entry* en;
+  ASSERT_FALSE(cache.disk_cache()->OpenEntry(kRangeGET_TransactionOK.url, &en));
+
+  RemoveMockTransaction(&kRangeGET_TransactionOK);
+}
+
+// Tests that we reject a range that doesn't match the content-length.
+TEST(HttpCache, RangeGET_InvalidResponse2) {
+  MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
+  std::string headers;
+
+  MockTransaction transaction(kRangeGET_TransactionOK);
+  transaction.handler = NULL;
+  transaction.response_headers = "Content-Range: bytes 40-49/80\n"
+                                 "Content-Length: 20\n";
+  AddMockTransaction(&transaction);
+  RunTransactionTestWithResponse(cache.http_cache(), transaction, &headers);
+
+  std::string expected(transaction.status);
+  expected.append("\n");
+  expected.append(transaction.response_headers);
+  EXPECT_EQ(expected, headers);
+
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  // Verify that we don't have a cached entry.
+  disk_cache::Entry* en;
+  ASSERT_FALSE(cache.disk_cache()->OpenEntry(kRangeGET_TransactionOK.url, &en));
+
+  RemoveMockTransaction(&kRangeGET_TransactionOK);
+}
+
+// Tests that if a server tells us conflicting information about a resource we
+// ignore the response.
+TEST(HttpCache, RangeGET_InvalidResponse3) {
+  MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
+  std::string headers;
+
+  MockTransaction transaction(kRangeGET_TransactionOK);
+  transaction.handler = NULL;
+  transaction.request_headers = "Range: bytes = 50-59\r\n";
+  std::string response_headers(transaction.response_headers);
+  response_headers.append("Content-Range: bytes 50-59/160\n");
+  transaction.response_headers = response_headers.c_str();
+  AddMockTransaction(&transaction);
+  RunTransactionTestWithResponse(cache.http_cache(), transaction, &headers);
+
+  EXPECT_TRUE(Verify206Response(headers, 50, 59));
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  RemoveMockTransaction(&transaction);
+  AddMockTransaction(&kRangeGET_TransactionOK);
+
+  // This transaction will report a resource size of 80 bytes, and we think it's
+  // 160 so we should ignore the response.
+  RunTransactionTestWithResponse(cache.http_cache(), kRangeGET_TransactionOK,
+                                 &headers);
+
+  EXPECT_TRUE(Verify206Response(headers, 40, 49));
+  EXPECT_EQ(2, cache.network_layer()->transaction_count());
+  EXPECT_EQ(1, cache.disk_cache()->open_count());
+  EXPECT_EQ(1, cache.disk_cache()->create_count());
+
+  // Verify that we cached the first response but not the second one.
+  disk_cache::Entry* en;
+  ASSERT_TRUE(cache.disk_cache()->OpenEntry(kRangeGET_TransactionOK.url, &en));
+  int64 cached_start = 0;
+  EXPECT_EQ(10, en->GetAvailableRange(40, 20, &cached_start));
+  EXPECT_EQ(50, cached_start);
+  en->Close();
+
+  RemoveMockTransaction(&kRangeGET_TransactionOK);
+}
+
+// Tests that we handle large range values properly.
+TEST(HttpCache, RangeGET_LargeValues) {
+  // We need a real sparse cache for this test.
+  disk_cache::Backend* disk_cache =
+      disk_cache::CreateInMemoryCacheBackend(1024 * 1024);
+  MockHttpCache cache(disk_cache);
+  cache.http_cache()->set_enable_range_support(true);
+  std::string headers;
+
+  MockTransaction transaction(kRangeGET_TransactionOK);
+  transaction.handler = NULL;
+  transaction.request_headers = "Range: bytes = 4294967288-4294967297\r\n";
+  transaction.response_headers =
+      "Content-Range: bytes 4294967288-4294967297/4294967299\n"
+      "Content-Length: 10\n";
+  AddMockTransaction(&transaction);
+  RunTransactionTestWithResponse(cache.http_cache(), transaction, &headers);
+
+  std::string expected(transaction.status);
+  expected.append("\n");
+  expected.append(transaction.response_headers);
+  EXPECT_EQ(expected, headers);
+
+  EXPECT_EQ(1, cache.network_layer()->transaction_count());
+
+  // Verify that we have a cached entry.
+  disk_cache::Entry* en;
+  ASSERT_TRUE(cache.disk_cache()->OpenEntry(kRangeGET_TransactionOK.url, &en));
+  en->Close();
+
+  RemoveMockTransaction(&kRangeGET_TransactionOK);
+}
+
 #ifdef NDEBUG
 // This test hits a NOTREACHED so it is a release mode only test.
 TEST(HttpCache, RangeGET_OK_LoadOnlyFromCache) {
diff --git a/net/http/http_response_headers.cc b/net/http/http_response_headers.cc
index 9cbd72f..4397939 100644
--- a/net/http/http_response_headers.cc
+++ b/net/http/http_response_headers.cc
@@ -765,7 +765,6 @@ bool HttpResponseHeaders::IsRedirect(std::string* location) const {
 bool HttpResponseHeaders::RequiresValidation(const Time& request_time,
                                              const Time& response_time,
                                              const Time& current_time) const {
-
   TimeDelta lifetime =
       GetFreshnessLifetime(response_time);
   if (lifetime == TimeDelta())
@@ -1032,6 +1031,7 @@ bool HttpResponseHeaders::GetContentRange(int64* first_byte_position,
                                           int64* instance_length) const {
   void* iter = NULL;
   std::string content_range_spec;
+  *first_byte_position = *last_byte_position = *instance_length = -1;
   if (!EnumerateHeader(&iter, "content-range", &content_range_spec))
     return false;
 
@@ -1069,11 +1069,8 @@ bool HttpResponseHeaders::GetContentRange(int64* first_byte_position,
   // Parse the byte-range-resp-spec part.
   std::string byte_range_resp_spec(byte_range_resp_spec_begin,
                                    byte_range_resp_spec_end);
-  // If byte-range-resp-spec == "*".
-  if (LowerCaseEqualsASCII(byte_range_resp_spec, "*")) {
-    *first_byte_position = -1;
-    *last_byte_position = -1;
-  } else {
+  // If byte-range-resp-spec != "*".
+  if (!LowerCaseEqualsASCII(byte_range_resp_spec, "*")) {
     size_t minus_position = byte_range_resp_spec.find('-');
     if (minus_position != std::string::npos) {
       // Obtain first-byte-pos.
@@ -1097,9 +1094,11 @@ bool HttpResponseHeaders::GetContentRange(int64* first_byte_position,
       ok &= StringToInt64(
           std::string(last_byte_pos_begin, last_byte_pos_end),
           last_byte_position);
-      if (!ok ||
-          *first_byte_position < 0 ||
-          *last_byte_position < 0 ||
+      if (!ok) {
+        *first_byte_position = *last_byte_position = -1;
+        return false;
+      }
+      if (*first_byte_position < 0 || *last_byte_position < 0 ||
           *first_byte_position > *last_byte_position)
         return false;
     } else {
@@ -1116,17 +1115,20 @@ bool HttpResponseHeaders::GetContentRange(int64* first_byte_position,
   HttpUtil::TrimLWS(&instance_length_begin, &instance_length_end);
 
   if (LowerCaseEqualsASCII(instance_length_begin, instance_length_end, "*")) {
-    *instance_length = -1;
+    return false;
   } else if (!StringToInt64(
                  std::string(instance_length_begin, instance_length_end),
                  instance_length)) {
-    return false;
-  } else if (*instance_length < 0 ||
-             *instance_length <
-                 *last_byte_position - *first_byte_position + 1) {
+    *instance_length = -1;
     return false;
   }
 
+  // We have all the values; let's verify that they make sense for a 206
+  // response.
+  if (*first_byte_position < 0 || *last_byte_position < 0 ||
+      *instance_length < 0 || *instance_length - 1 < *last_byte_position)
+    return false;
+
   return true;
 }
 
diff --git a/net/http/http_response_headers.h b/net/http/http_response_headers.h
index 547370d..f1512b4 100644
--- a/net/http/http_response_headers.h
+++ b/net/http/http_response_headers.h
@@ -23,8 +23,8 @@ class TimeDelta;
 namespace net {
 
 // HttpResponseHeaders: parses and holds HTTP response headers.
-class HttpResponseHeaders :
-    public base::RefCountedThreadSafe<HttpResponseHeaders> {
+class HttpResponseHeaders
+    : public base::RefCountedThreadSafe<HttpResponseHeaders> {
  public:
   // Parses the given raw_headers.  raw_headers should be formatted thus:
   // includes the http status response line, each line is \0-terminated, and
@@ -211,8 +211,8 @@ class HttpResponseHeaders :
   // no such header in the response.
   int64 GetContentLength() const;
 
-  // Extracts the values in Content-Range header, if the header exists and is
-  // well formatted returns true, else returns false.
+  // Extracts the values in a Content-Range header and returns true if they are
+  // valid for a 206 response; otherwise returns false.
   // The following values will be outputted:
   // |*first_byte_position| = inclusive position of the first byte of the range
   // |*last_byte_position| = inclusive position of the last byte of the range
diff --git a/net/http/http_response_headers_unittest.cc b/net/http/http_response_headers_unittest.cc
index 9f0cfde..e18dc1f 100644
--- a/net/http/http_response_headers_unittest.cc
+++ b/net/http/http_response_headers_unittest.cc
@@ -253,7 +253,7 @@ TEST(HttpResponseHeadersTest, NormalizeHeadersOfWhitepace) {
     "HTTP/1.0 200 OK\n",
 
     200,
-    HttpVersion(0,0), // Parse error
+    HttpVersion(0,0),  // Parse error
     HttpVersion(1,0)
   };
   TestCommon(test);
@@ -1157,6 +1157,13 @@ TEST(HttpResponseHeaders, GetContentRange) {
     { "HTTP/1.1 206 Partial Content\n"
       "Content-Range: \t   bytes \t  0    -   50   /   5   1",
       false,
+      0,
+      50,
+      -1
+    },
+    { "HTTP/1.1 206 Partial Content\n"
+      "Content-Range: \t   bytes \t  0    -   5 0   /   51",
+      false,
       -1,
       -1,
       -1
@@ -1164,34 +1171,34 @@ TEST(HttpResponseHeaders, GetContentRange) {
     { "HTTP/1.1 206 Partial Content\n"
       "Content-Range: bytes 50-0/51",
       false,
-      -1,
-      -1,
+      50,
+      0,
       -1
     },
     { "HTTP/1.1 416 Requested range not satisfiable\n"
-      "Content-Range: bytes */*",
-      true,
+      "Content-Range: bytes * /*",
+      false,
       -1,
       -1,
       -1
     },
     { "HTTP/1.1 416 Requested range not satisfiable\n"
       "Content-Range: bytes *   /    *   ",
-      true,
+      false,
       -1,
       -1,
       -1
     },
     { "HTTP/1.1 206 Partial Content\n"
       "Content-Range: bytes 0-50/*",
-      true,
+      false,
       0,
       50,
       -1
     },
     { "HTTP/1.1 206 Partial Content\n"
       "Content-Range: bytes 0-50  /    * ",
-      true,
+      false,
       0,
       50,
       -1
@@ -1206,15 +1213,29 @@ TEST(HttpResponseHeaders, GetContentRange) {
     { "HTTP/1.1 206 Partial Content\n"
       "Content-Range: bytes 0-10000000000/10000000000",
       false,
-      -1,
-      -1,
-      -1
+      0,
+      10000000000ll,
+      10000000000ll
+    },
+    // 64 bits wraparound.
+    { "HTTP/1.1 206 Partial Content\n"
+      "Content-Range: bytes 0 - 9223372036854775807 / 100",
+      false,
+      0,
+      kint64max,
+      100
+    },
+    // 64 bits wraparound.
+    { "HTTP/1.1 206 Partial Content\n"
+      "Content-Range: bytes 0 - 100 / -9223372036854775808",
+      false,
+      0,
+      100,
+      kint64min
     },
-    // The following header is invalid for response code of 206, this should be
-    // verified by the user.
     { "HTTP/1.1 206 Partial Content\n"
       "Content-Range: bytes */50",
-      true,
+      false,
       -1,
       -1,
       50
@@ -1222,16 +1243,16 @@ TEST(HttpResponseHeaders, GetContentRange) {
     { "HTTP/1.1 206 Partial Content\n"
       "Content-Range: bytes 0-50/10",
       false,
-      -1,
-      -1,
-      -1
+      0,
+      50,
+      10
     },
     { "HTTP/1.1 206 Partial Content\n"
       "Content-Range: bytes 0-50/-10",
       false,
-      -1,
-      -1,
-      -1
+      0,
+      50,
+      -10
     },
     { "HTTP/1.1 206 Partial Content\n"
       "Content-Range: bytes 0-0/1",
@@ -1269,15 +1290,8 @@ TEST(HttpResponseHeaders, GetContentRange) {
       -1
     },
     { "HTTP/1.1 206 Partial Content\n"
-      "Content-Range: bytes 0-40000000000000000000/40000000000000000001",
-      false,
-      -1,
-      -1,
-      -1
-    },
-    { "HTTP/1.1 206 Partial Content\n"
       "Content-Range: bytes 0-1233/*",
-      true,
+      false,
       0,
       1233,
       -1
@@ -1303,11 +1317,9 @@ TEST(HttpResponseHeaders, GetContentRange) {
                                                 &last_byte_position,
                                                 &instance_size);
     EXPECT_EQ(tests[i].expected_return_value, return_value);
-    if (return_value) {
-      EXPECT_EQ(tests[i].expected_first_byte_position, first_byte_position);
-      EXPECT_EQ(tests[i].expected_last_byte_position, last_byte_position);
-      EXPECT_EQ(tests[i].expected_instance_size, instance_size);
-    }
+    EXPECT_EQ(tests[i].expected_first_byte_position, first_byte_position);
+    EXPECT_EQ(tests[i].expected_last_byte_position, last_byte_position);
+    EXPECT_EQ(tests[i].expected_instance_size, instance_size);
   }
 }
 
diff --git a/net/http/partial_data.cc b/net/http/partial_data.cc
index b61d9be..57985e7 100644
--- a/net/http/partial_data.cc
+++ b/net/http/partial_data.cc
@@ -192,6 +192,10 @@ bool PartialData::ResponseHeadersOK(const HttpResponseHeaders* headers) {
   if (total_length <= 0)
     return false;
 
+  int64 content_length = headers->GetContentLength();
+  if (content_length != end - start + 1)
+    return false;
+
   if (!resource_size_) {
     // First response. Update our values with the ones provided by the server.
     resource_size_ = total_length;