CacheStorage: Refactor opaque padding.

This CL refactors how we generate and store opaque response padding:

* Padding values are now generated immediately in fetch().
* Padding values are associated with the Response and follow it.
* Network loaded responses get a purely random pad.
* Http cache loaded responses get a hashed padding value.
* CacheStorage now stores padding values in each entry.
* CacheStorage entries with side data for code cache have a separate,
  additional padding value added.
* Many additional tests.

Bug: 1143526
Change-Id: I40b094097b64be7bab8899acad8b9baffe304d33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2590076
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#849608}

content/browser/appcache/appcache_backfillers.cc

@@ -7,7 +7,7 @@
 #include "content/browser/appcache/appcache_update_job.h"
 #include "net/http/http_request_headers.h"
 #include "sql/statement.h"
-#include "storage/browser/quota/padding_key.h"
+#include "storage/common/quota/padding_key.h"
 #include "url/gurl.h"
 
 namespace content {
@@ -18,9 +18,7 @@ int64_t ComputeEntryPaddingSize(std::string response_url,
                                 std::string manifest_url) {
   if (GURL(response_url).GetOrigin() == GURL(manifest_url).GetOrigin())
     return 0;
-  return storage::ComputeResponsePadding(
-      response_url, storage::GetDefaultPaddingKey(), /*has_metadata=*/false,
-      /*loaded_with_credentials=*/false, net::HttpRequestHeaders::kGetMethod);
+  return storage::ComputeRandomResponsePadding();
 }
 
 // Iterates over each Cache record; execute |callable| on each iteration.

content/browser/appcache/appcache_database.cc

@@ -19,7 +19,6 @@
 #include "sql/meta_table.h"
 #include "sql/statement.h"
 #include "sql/transaction.h"
-#include "storage/browser/quota/padding_key.h"
 #include "third_party/blink/public/common/features.h"
 
 namespace content {

content/browser/appcache/appcache_update_job.cc

@@ -26,7 +26,7 @@
 #include "net/base/net_errors.h"
 #include "net/base/request_priority.h"
 #include "net/http/http_request_headers.h"
-#include "storage/browser/quota/padding_key.h"
+#include "storage/common/quota/padding_key.h"
 #include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/mojom/appcache/appcache.mojom.h"
 #include "third_party/blink/public/mojom/devtools/console_message.mojom.h"
@@ -207,10 +207,7 @@ int64_t ComputeAppCacheResponsePadding(const GURL& response_url,
   if (response_url.GetOrigin() == manifest_url.GetOrigin())
     return 0;
 
-  return storage::ComputeResponsePadding(
-      response_url.spec(), storage::GetDefaultPaddingKey(),
-      /*has_metadata=*/false, /*loaded_with_credentials=*/false,
-      net::HttpRequestHeaders::kGetMethod);
+  return storage::ComputeRandomResponsePadding();
 }
 
 }  // namespace

content/browser/cache_storage/cache_storage.proto

@@ -56,6 +56,8 @@ message CacheResponse {
   optional bool was_fetched_via_spdy = 12;
   optional string mime_type = 13;
   optional string request_method = 14;
+  optional int64 padding = 15;
+  optional int64 side_data_padding = 16;
 }
 
 message CacheMetadata {

content/browser/cache_storage/cache_storage_cache_unittest.cc

@@ -53,6 +53,7 @@
 #include "storage/browser/test/fake_blob.h"
 #include "storage/browser/test/mock_quota_manager_proxy.h"
 #include "storage/browser/test/mock_special_storage_policy.h"
+#include "storage/common/quota/padding_key.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom.h"
 #include "url/origin.h"
@@ -649,6 +650,18 @@ class CacheStorageCacheTest : public testing::Test {
     return response;
   }
 
+  blink::mojom::FetchAPIResponsePtr CreateOpaqueResponse() {
+    auto response = CreateBlobBodyResponse();
+    response->response_type = network::mojom::FetchResponseType::kOpaque;
+    response->response_time = base::Time::Now();
+
+    // CacheStorage depends on fetch to provide the opaque response padding
+    // value now.  We prepolute a padding value here to simulate that.
+    response->padding = 10;
+
+    return response;
+  }
+
   blink::mojom::FetchAPIResponsePtr CreateBlobBodyResponseWithQuery() {
     blink::mojom::FetchAPIResponsePtr response = CreateBlobBodyResponse();
     response->url_list = {BodyUrlWithQuery()};
@@ -659,7 +672,7 @@ class CacheStorageCacheTest : public testing::Test {
   blink::mojom::FetchAPIResponsePtr CreateNoBodyResponse() {
     return blink::mojom::FetchAPIResponse::New(
         std::vector<GURL>({NoBodyUrl()}), 200, "OK",
-        network::mojom::FetchResponseType::kDefault,
+        network::mojom::FetchResponseType::kDefault, /*padding=*/0,
         network::mojom::FetchResponseSource::kUnspecified,
         base::flat_map<std::string, std::string>(kHeaders.cbegin(),
                                                  kHeaders.cend()),
@@ -957,6 +970,8 @@ class CacheStorageCacheTest : public testing::Test {
   bool TestResponseType(network::mojom::FetchResponseType response_type) {
     blink::mojom::FetchAPIResponsePtr body_response = CreateBlobBodyResponse();
     body_response->response_type = response_type;
+    if (storage::ShouldPadResponseType(response_type))
+      body_response->padding = 10;
     EXPECT_TRUE(Put(body_request_, std::move(body_response)));
     EXPECT_TRUE(Match(body_request_));
     EXPECT_TRUE(Delete(body_request_));
@@ -2203,9 +2218,6 @@ TEST_F(CacheStorageCacheTest, VerifyOpaqueSizePadding) {
   blink::mojom::FetchAPIResponsePtr non_opaque_response =
       CreateBlobBodyResponse();
   non_opaque_response->response_time = response_time;
-  EXPECT_EQ(0, LegacyCacheStorageCache::CalculateResponsePadding(
-                   *non_opaque_response, CreateTestPaddingKey().get(),
-                   0 /* side_data_size */));
   EXPECT_TRUE(Put(non_opaque_request, std::move(non_opaque_response)));
   int64_t unpadded_no_data_cache_size = Size();
 
@@ -2223,9 +2235,6 @@ TEST_F(CacheStorageCacheTest, VerifyOpaqueSizePadding) {
   blink::mojom::FetchAPIResponsePtr non_opaque_response_clone =
       CreateBlobBodyResponse();
   non_opaque_response_clone->response_time = response_time;
-  EXPECT_EQ(0, LegacyCacheStorageCache::CalculateResponsePadding(
-                   *non_opaque_response_clone, CreateTestPaddingKey().get(),
-                   unpadded_side_data_size));
 
   // Now write an identically sized opaque response.
   blink::mojom::FetchAPIRequestPtr opaque_request =
@@ -2234,14 +2243,10 @@ TEST_F(CacheStorageCacheTest, VerifyOpaqueSizePadding) {
   // Same URL length means same cache sizes (ignoring padding).
   EXPECT_EQ(opaque_request->url.spec().length(),
             non_opaque_request->url.spec().length());
-  blink::mojom::FetchAPIResponsePtr opaque_response(CreateBlobBodyResponse());
-  opaque_response->response_type = network::mojom::FetchResponseType::kOpaque;
+  blink::mojom::FetchAPIResponsePtr opaque_response(CreateOpaqueResponse());
   opaque_response->response_time = response_time;
 
   EXPECT_TRUE(Put(opaque_request, std::move(opaque_response)));
-  // This test is fragile. Right now it deterministically adds non-zero padding.
-  // But if the url, padding key, or padding algorithm change it might become
-  // zero.
   int64_t size_after_opaque_put = Size();
   int64_t opaque_padding = size_after_opaque_put -
                            2 * unpadded_no_data_cache_size -
@@ -2265,39 +2270,15 @@ TEST_F(CacheStorageCacheTest, VerifyOpaqueSizePadding) {
   // And delete the opaque response entirely.
   EXPECT_TRUE(Delete(opaque_request));
   EXPECT_EQ(unpadded_total_resource_size, Size());
-
-  // Now write an identically sized opaque response with the
-  // loaded_with_credentials flag set.
-  blink::mojom::FetchAPIRequestPtr credentialed_opaque_request =
-      BackgroundFetchSettledFetch::CloneRequest(non_opaque_request);
-  credentialed_opaque_request->url = GURL("http://example.com/opaque.html");
-  // Same URL length means same cache sizes (ignoring padding).
-  EXPECT_EQ(credentialed_opaque_request->url.spec().length(),
-            non_opaque_request->url.spec().length());
-  blink::mojom::FetchAPIResponsePtr credentialed_opaque_response(
-      CreateBlobBodyResponse());
-  credentialed_opaque_response->response_type =
-      network::mojom::FetchResponseType::kOpaque;
-  credentialed_opaque_response->response_time = response_time;
-  credentialed_opaque_response->loaded_with_credentials = true;
-
-  EXPECT_TRUE(Put(credentialed_opaque_request,
-                  std::move(credentialed_opaque_response)));
-
-  int64_t size_after_credentialed_opaque_put = Size();
-  int64_t credentialed_opaque_padding = size_after_credentialed_opaque_put -
-                                        2 * unpadded_no_data_cache_size -
-                                        unpadded_side_data_size;
-  ASSERT_NE(credentialed_opaque_padding, opaque_padding);
 }
 
 TEST_F(CacheStorageCacheTest, TestDifferentOpaqueSideDataSizes) {
   blink::mojom::FetchAPIRequestPtr request =
       BackgroundFetchSettledFetch::CloneRequest(body_request_);
-  blink::mojom::FetchAPIResponsePtr response(CreateBlobBodyResponse());
-  response->response_type = network::mojom::FetchResponseType::kOpaque;
-  base::Time response_time(base::Time::Now());
-  response->response_time = response_time;
+  blink::mojom::FetchAPIResponsePtr response(CreateOpaqueResponse());
+
+  auto response_time = response->response_time;
+
   EXPECT_TRUE(Put(request, std::move(response)));
   int64_t opaque_cache_size_no_side_data = Size();
 
@@ -2309,16 +2290,15 @@ TEST_F(CacheStorageCacheTest, TestDifferentOpaqueSideDataSizes) {
   int64_t opaque_cache_size_with_side_data = Size();
   EXPECT_NE(opaque_cache_size_with_side_data, opaque_cache_size_no_side_data);
 
-  // Write side data of a different size. The size should not affect the padding
-  // at all.
+  // Write side data of a different size. The padding should change.
   const std::string large_side_data(2048, 'X');
   EXPECT_NE(large_side_data.length(), small_side_data.length());
   scoped_refptr<net::IOBuffer> buffer2 =
       base::MakeRefCounted<net::StringIOBuffer>(large_side_data);
   EXPECT_TRUE(WriteSideData(request->url, response_time, buffer2,
                             large_side_data.length()));
   int side_data_delta = large_side_data.length() - small_side_data.length();
-  EXPECT_EQ(opaque_cache_size_with_side_data + side_data_delta, Size());
+  EXPECT_NE(opaque_cache_size_with_side_data + side_data_delta, Size());
 }
 
 TEST_F(CacheStorageCacheTest, TestDoubleOpaquePut) {
@@ -2327,16 +2307,14 @@ TEST_F(CacheStorageCacheTest, TestDoubleOpaquePut) {
 
   base::Time response_time(base::Time::Now());
 
-  blink::mojom::FetchAPIResponsePtr response(CreateBlobBodyResponse());
-  response->response_type = network::mojom::FetchResponseType::kOpaque;
+  blink::mojom::FetchAPIResponsePtr response(CreateOpaqueResponse());
   response->response_time = response_time;
   EXPECT_TRUE(Put(request, std::move(response)));
   int64_t size_after_first_put = Size();
 
   blink::mojom::FetchAPIRequestPtr request2 =
       BackgroundFetchSettledFetch::CloneRequest(body_request_);
-  blink::mojom::FetchAPIResponsePtr response2(CreateBlobBodyResponse());
-  response2->response_type = network::mojom::FetchResponseType::kOpaque;
+  blink::mojom::FetchAPIResponsePtr response2(CreateOpaqueResponse());
   response2->response_time = response_time;
   EXPECT_TRUE(Put(request2, std::move(response2)));
 

content/browser/cache_storage/cache_storage_histogram_utils.h

@@ -37,7 +37,8 @@ enum class ErrorStorageType {
   kCreateBackendDidCreateFailed = 22,
   kStorageGetAllMatchedEntriesBackendClosed = 23,
   kStorageHandleNull = 24,
-  kMaxValue = kStorageHandleNull,
+  kWriteSideDataDidWriteMetadataWrongBytes = 25,
+  kMaxValue = kWriteSideDataDidWriteMetadataWrongBytes,
 };
 
 blink::mojom::CacheStorageError MakeErrorStorage(ErrorStorageType type);