Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(529)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: components/safe_browsing_db/v4_store_unittest.cc

Issue 2406373003: Small: Delay checksum only on ReadFromDisk, not in FullUpdate cases (Closed)
Patch Set: Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« components/safe_browsing_db/v4_store.cc ('K') | « components/safe_browsing_db/v4_store.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/safe_browsing_db/v4_store_unittest.cc
diff --git a/components/safe_browsing_db/v4_store_unittest.cc b/components/safe_browsing_db/v4_store_unittest.cc
index 73cd785b06caf5e05a02f2b06f4d7e83ad2a092e..06ee1c530df5e82797d7a14e11b538de591f40a0 100644
--- a/components/safe_browsing_db/v4_store_unittest.cc
+++ b/components/safe_browsing_db/v4_store_unittest.cc
@@ -57,12 +57,18 @@ class V4StoreTest : public PlatformTest {
file_format_string.size());
}
- void UpdatedStoreReadyAfterRiceRemovals(bool* called_back,
- std::unique_ptr<V4Store> new_store) {
+ void UpdatedStoreReady(bool* called_back,
+ bool expect_store,
+ std::unique_ptr<V4Store> store) {
*called_back = true;
- EXPECT_EQ(2u, new_store->hash_prefix_map_.size());
- EXPECT_EQ("22222", new_store->hash_prefix_map_[5]);
- EXPECT_EQ("abcd", new_store->hash_prefix_map_[4]);
+ if (expect_store) {
+ ASSERT_TRUE(store);
+ EXPECT_EQ(2u, store->hash_prefix_map_.size());
+ EXPECT_EQ("22222", store->hash_prefix_map_[5]);
+ EXPECT_EQ("abcd", store->hash_prefix_map_[4]);
+ } else {
+ ASSERT_FALSE(store);
+ }
}
base::ScopedTempDir temp_dir_;
@@ -218,7 +224,7 @@ TEST_F(V4StoreTest, TestMergeUpdatesWithSameSizesInEachMap) {
crypto::kSHA256Length);
EXPECT_EQ(APPLY_UPDATE_SUCCESS,
store.MergeUpdate(prefix_map_old, prefix_map_additions, nullptr,
- expected_checksum));
+ expected_checksum, false));
const HashPrefixMap& prefix_map = store.hash_prefix_map_;
EXPECT_EQ(2u, prefix_map.size());
@@ -257,7 +263,7 @@ TEST_F(V4StoreTest, TestMergeUpdatesWithDifferentSizesInEachMap) {
crypto::kSHA256Length);
EXPECT_EQ(APPLY_UPDATE_SUCCESS,
store.MergeUpdate(prefix_map_old, prefix_map_additions, nullptr,
- expected_checksum));
+ expected_checksum, false));
const HashPrefixMap& prefix_map = store.hash_prefix_map_;
EXPECT_EQ(2u, prefix_map.size());
@@ -289,7 +295,7 @@ TEST_F(V4StoreTest, TestMergeUpdatesOldMapRunsOutFirst) {
crypto::kSHA256Length);
EXPECT_EQ(APPLY_UPDATE_SUCCESS,
store.MergeUpdate(prefix_map_old, prefix_map_additions, nullptr,
- expected_checksum));
+ expected_checksum, false));
const HashPrefixMap& prefix_map = store.hash_prefix_map_;
EXPECT_EQ(1u, prefix_map.size());
@@ -318,7 +324,7 @@ TEST_F(V4StoreTest, TestMergeUpdatesAdditionsMapRunsOutFirst) {
crypto::kSHA256Length);
EXPECT_EQ(APPLY_UPDATE_SUCCESS,
store.MergeUpdate(prefix_map_old, prefix_map_additions, nullptr,
- expected_checksum));
+ expected_checksum, false));
const HashPrefixMap& prefix_map = store.hash_prefix_map_;
EXPECT_EQ(1u, prefix_map.size());
@@ -343,7 +349,7 @@ TEST_F(V4StoreTest, TestMergeUpdatesFailsForRepeatedHashPrefix) {
std::string expected_checksum;
EXPECT_EQ(ADDITIONS_HAS_EXISTING_PREFIX_FAILURE,
store.MergeUpdate(prefix_map_old, prefix_map_additions, nullptr,
- expected_checksum));
+ expected_checksum, false));
}
TEST_F(V4StoreTest, TestMergeUpdatesFailsWhenRemovalsIndexTooLarge) {
@@ -363,7 +369,7 @@ TEST_F(V4StoreTest, TestMergeUpdatesFailsWhenRemovalsIndexTooLarge) {
std::string expected_checksum;
EXPECT_EQ(REMOVALS_INDEX_TOO_LARGE_FAILURE,
store.MergeUpdate(prefix_map_old, prefix_map_additions,
- &raw_removals, expected_checksum));
+ &raw_removals, expected_checksum, false));
}
TEST_F(V4StoreTest, TestMergeUpdatesRemovesOnlyElement) {
@@ -386,7 +392,7 @@ TEST_F(V4StoreTest, TestMergeUpdatesRemovesOnlyElement) {
crypto::kSHA256Length);
EXPECT_EQ(APPLY_UPDATE_SUCCESS,
store.MergeUpdate(prefix_map_old, prefix_map_additions,
- &raw_removals, expected_checksum));
+ &raw_removals, expected_checksum, false));
const HashPrefixMap& prefix_map = store.hash_prefix_map_;
// The size is 2 since we reserve space anyway.
@@ -415,7 +421,7 @@ TEST_F(V4StoreTest, TestMergeUpdatesRemovesFirstElement) {
crypto::kSHA256Length);
EXPECT_EQ(APPLY_UPDATE_SUCCESS,
store.MergeUpdate(prefix_map_old, prefix_map_additions,
- &raw_removals, expected_checksum));
+ &raw_removals, expected_checksum, false));
const HashPrefixMap& prefix_map = store.hash_prefix_map_;
// The size is 2 since we reserve space anyway.
@@ -443,7 +449,7 @@ TEST_F(V4StoreTest, TestMergeUpdatesRemovesMiddleElement) {
crypto::kSHA256Length);
EXPECT_EQ(APPLY_UPDATE_SUCCESS,
store.MergeUpdate(prefix_map_old, prefix_map_additions,
- &raw_removals, expected_checksum));
+ &raw_removals, expected_checksum, false));
const HashPrefixMap& prefix_map = store.hash_prefix_map_;
// The size is 2 since we reserve space anyway.
@@ -470,7 +476,7 @@ TEST_F(V4StoreTest, TestMergeUpdatesRemovesLastElement) {
crypto::kSHA256Length);
EXPECT_EQ(APPLY_UPDATE_SUCCESS,
store.MergeUpdate(prefix_map_old, prefix_map_additions,
- &raw_removals, expected_checksum));
+ &raw_removals, expected_checksum, false));
const HashPrefixMap& prefix_map = store.hash_prefix_map_;
// The size is 2 since we reserve space anyway.
@@ -500,7 +506,7 @@ TEST_F(V4StoreTest, TestMergeUpdatesRemovesWhenOldHasDifferentSizes) {
crypto::kSHA256Length);
EXPECT_EQ(APPLY_UPDATE_SUCCESS,
store.MergeUpdate(prefix_map_old, prefix_map_additions,
- &raw_removals, expected_checksum));
+ &raw_removals, expected_checksum, false));
const HashPrefixMap& prefix_map = store.hash_prefix_map_;
// The size is 2 since we reserve space anyway.
@@ -532,7 +538,7 @@ TEST_F(V4StoreTest, TestMergeUpdatesRemovesMultipleAcrossDifferentSizes) {
crypto::kSHA256Length);
EXPECT_EQ(APPLY_UPDATE_SUCCESS,
store.MergeUpdate(prefix_map_old, prefix_map_additions,
- &raw_removals, expected_checksum));
+ &raw_removals, expected_checksum, false));
const HashPrefixMap& prefix_map = store.hash_prefix_map_;
// The size is 2 since we reserve space anyway.
@@ -712,7 +718,7 @@ TEST_F(V4StoreTest, TestRemovalsWithRiceEncodingSucceeds) {
crypto::kSHA256Length);
EXPECT_EQ(APPLY_UPDATE_SUCCESS,
store.MergeUpdate(prefix_map_old, prefix_map_additions, nullptr,
- expected_checksum));
+ expected_checksum, false));
// At this point, the store map looks like this:
// 4: 1111abcdefgh
@@ -732,8 +738,8 @@ TEST_F(V4StoreTest, TestRemovalsWithRiceEncodingSucceeds) {
bool called_back = false;
UpdatedStoreReadyCallback store_ready_callback =
- base::Bind(&V4StoreTest::UpdatedStoreReadyAfterRiceRemovals,
- base::Unretained(this), &called_back);
+ base::Bind(&V4StoreTest::UpdatedStoreReady, base::Unretained(this),
+ &called_back, true);
EXPECT_FALSE(base::PathExists(store.store_path_));
store.ApplyUpdate(std::move(lur), task_runner_, store_ready_callback);
EXPECT_TRUE(base::PathExists(store.store_path_));
@@ -759,7 +765,8 @@ TEST_F(V4StoreTest, TestMergeUpdatesFailsChecksum) {
V4Store::AddUnlumpedHashes(4, "2222", &prefix_map_old));
EXPECT_EQ(CHECKSUM_MISMATCH_FAILURE,
V4Store(task_runner_, store_path_)
- .MergeUpdate(prefix_map_old, HashPrefixMap(), nullptr, "aawc"));
+ .MergeUpdate(prefix_map_old, HashPrefixMap(), nullptr, "aawc",
+ false));
}
TEST_F(V4StoreTest, TestChecksumErrorOnStartup) {
@@ -814,4 +821,30 @@ TEST_F(V4StoreTest, WriteToDiskFails) {
V4Store(task_runner_, temp_dir_.GetPath()).WriteToDisk(Checksum()));
}
+TEST_F(V4StoreTest, FullUpdateFailsChecksumSynchronously) {
+ V4Store store(task_runner_, store_path_);
+ bool called_back = false;
+ UpdatedStoreReadyCallback store_ready_callback =
+ base::Bind(&V4StoreTest::UpdatedStoreReady, base::Unretained(this),
+ &called_back, false);
+ EXPECT_FALSE(base::PathExists(store.store_path_));
+
+ // Now create a response with invalid checksum.
+ std::unique_ptr<ListUpdateResponse> lur(new ListUpdateResponse);
+ lur->set_response_type(ListUpdateResponse::FULL_UPDATE);
+ lur->mutable_checksum()->set_sha256(std::string(crypto::kSHA256Length, 0));
+ store.ApplyUpdate(std::move(lur), task_runner_, store_ready_callback);
+ // The update should fail synchronously and not create a store file.
+ EXPECT_FALSE(base::PathExists(store.store_path_));
+
+ // Run everything on the task runner to ensure there are no pending tasks.
+ task_runner_->RunPendingTasks();
+ base::RunLoop().RunUntilIdle();
+
+ // This ensures that the callback was called.
+ EXPECT_TRUE(called_back);
+ // Ensure that the file is still not created.
+ EXPECT_FALSE(base::PathExists(store.store_path_));
+}
+
} // namespace safe_browsing
« components/safe_browsing_db/v4_store.cc ('K') | « components/safe_browsing_db/v4_store.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698