[code cache] Return empty string for secondary code cache keys
- Changes DCHECKs to tests for no prefix or separator. In that case,
return an empty string for the URL.
- Calling code is changed to test for an empty string before calling
the URL predicate.
Bug: chromium:1039740
Change-Id: If9e5216ff8e0ed6f952c7cc79bc4bd701ec069b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2057385
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741978}
diff --git a/content/browser/browsing_data/conditional_cache_deletion_helper.cc b/content/browser/browsing_data/conditional_cache_deletion_helper.cc
index 62b51bf..0b1fd00 100644
--- a/content/browser/browsing_data/conditional_cache_deletion_helper.cc
+++ b/content/browser/browsing_data/conditional_cache_deletion_helper.cc
@@ -23,9 +23,10 @@
const disk_cache::Entry* entry) {
std::string url = entry->GetKey();
if (!get_url_from_key.is_null())
- url = get_url_from_key.Run(entry->GetKey());
+ url = get_url_from_key.Run(url);
return (entry->GetLastUsed() >= begin_time &&
- entry->GetLastUsed() < end_time && url_predicate.Run(GURL(url)));
+ entry->GetLastUsed() < end_time && !url.empty() &&
+ url_predicate.Run(GURL(url)));
}
} // namespace
diff --git a/content/browser/code_cache/generated_code_cache.cc b/content/browser/code_cache/generated_code_cache.cc
index fa19ef61..dda24df 100644
--- a/content/browser/code_cache/generated_code_cache.cc
+++ b/content/browser/code_cache/generated_code_cache.cc
@@ -172,12 +172,15 @@
std::string GeneratedCodeCache::GetResourceURLFromKey(const std::string& key) {
constexpr size_t kPrefixStringLen = base::size(kPrefix) - 1;
- // Only expect valid keys. All valid keys have a prefix and a separator.
- DCHECK_GE(key.length(), kPrefixStringLen);
- DCHECK_NE(key.find(kSeparator), std::string::npos);
+ // |key| may not have a prefix and separator (e.g. for deduplicated entries).
+ // In that case, return an empty string.
+ const size_t separator_index = key.find(kSeparator);
+ if (key.length() < kPrefixStringLen || separator_index == std::string::npos) {
+ return std::string();
+ }
std::string resource_url =
- key.substr(kPrefixStringLen, key.find(kSeparator) - kPrefixStringLen);
+ key.substr(kPrefixStringLen, separator_index - kPrefixStringLen);
return resource_url;
}
diff --git a/content/browser/code_cache/generated_code_cache.h b/content/browser/code_cache/generated_code_cache.h
index 6d6d033..c8770b98 100644
--- a/content/browser/code_cache/generated_code_cache.h
+++ b/content/browser/code_cache/generated_code_cache.h
@@ -67,7 +67,7 @@
// Returns the resource URL from the key. The key has the format prefix +
// resource URL + separator + requesting origin. This function extracts and
- // returns resource URL from the key.
+ // returns resource URL from the key, or the empty string if key is invalid.
static std::string GetResourceURLFromKey(const std::string& key);
// Creates a GeneratedCodeCache with the specified path and the maximum size.
diff --git a/content/browser/code_cache/generated_code_cache_unittest.cc b/content/browser/code_cache/generated_code_cache_unittest.cc
index fecfedbf..ba4a099 100644
--- a/content/browser/code_cache/generated_code_cache_unittest.cc
+++ b/content/browser/code_cache/generated_code_cache_unittest.cc
@@ -133,6 +133,28 @@
constexpr char GeneratedCodeCacheTest::kInitialData[];
const size_t GeneratedCodeCacheTest::kMaxSizeInBytes;
+TEST_F(GeneratedCodeCacheTest, GetResourceURLFromKey) {
+ // These must be kept in sync with the values in generated_code_cache.cc.
+ constexpr char kPrefix[] = "_key";
+ constexpr char kSeparator[] = " \n";
+
+ // Test that we correctly extract the resource URL from a key.
+ std::string key(kPrefix);
+ key.append(kInitialUrl);
+ key.append(kSeparator);
+ key.append(kInitialOrigin);
+
+ EXPECT_EQ(GeneratedCodeCache::GetResourceURLFromKey(key), kInitialUrl);
+
+ // Invalid key formats should return the empty string.
+ ASSERT_TRUE(GeneratedCodeCache::GetResourceURLFromKey("").empty());
+ ASSERT_TRUE(GeneratedCodeCache::GetResourceURLFromKey("foobar").empty());
+ ASSERT_TRUE(
+ GeneratedCodeCache::GetResourceURLFromKey(
+ "43343250B630900F20597168708E14F17A6263F5251FCA10746EA7BDEA881085")
+ .empty());
+}
+
TEST_F(GeneratedCodeCacheTest, CheckResponseTime) {
GURL url(kInitialUrl);
GURL origin_lock = GURL(kInitialOrigin);
