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

Unified Diff: sync/internal_api/sync_manager_impl_unittest.cc

Issue 1161463005: [Sync] Don't crash for encryption errors (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address final comments Created 5 years, 6 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
« no previous file with comments | « sync/internal_api/sync_encryption_handler_impl.cc ('k') | sync/internal_api/syncapi_internal.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: sync/internal_api/sync_manager_impl_unittest.cc
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index 41b2e2df4b812fe06aa8eac08a32809131e66088..8bab47a49488b875fcbbc8690b4fc5bc766639c8 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -69,7 +69,7 @@
#include "sync/test/fake_encryptor.h"
#include "sync/util/cryptographer.h"
#include "sync/util/extensions_activity.h"
-#include "sync/util/test_unrecoverable_error_handler.h"
+#include "sync/util/mock_unrecoverable_error_handler.h"
#include "sync/util/time.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -836,7 +836,8 @@ class SyncManagerTest : public testing::Test,
};
SyncManagerTest()
- : sync_manager_("Test sync manager") {
+ : sync_manager_("Test sync manager"),
+ mock_unrecoverable_error_handler_(NULL) {
switches_.encryption_method =
InternalComponentsFactory::ENCRYPTION_KEYSTORE;
}
@@ -886,7 +887,8 @@ class SyncManagerTest : public testing::Test,
args.invalidator_client_id = "fake_invalidator_client_id";
args.internal_components_factory.reset(GetFactory());
args.encryptor = &encryptor_;
- args.unrecoverable_error_handler.reset(new TestUnrecoverableErrorHandler);
+ mock_unrecoverable_error_handler_ = new MockUnrecoverableErrorHandler();
+ args.unrecoverable_error_handler.reset(mock_unrecoverable_error_handler_);
args.cancelation_signal = &cancelation_signal_;
sync_manager_.Init(&args);
@@ -1080,6 +1082,12 @@ class SyncManagerTest : public testing::Test,
sync_manager_.GetEncryptionHandler()->GetPassphraseType());
}
+ bool HasUnrecoverableError() {
+ if (mock_unrecoverable_error_handler_)
+ return mock_unrecoverable_error_handler_->invocation_count() > 0;
+ return false;
+ }
+
private:
// Needed by |sync_manager_|.
base::MessageLoop message_loop_;
@@ -1099,6 +1107,9 @@ class SyncManagerTest : public testing::Test,
StrictMock<SyncEncryptionHandlerObserverMock> encryption_observer_;
InternalComponentsFactory::Switches switches_;
InternalComponentsFactory::StorageOption storage_used_;
+
+ // Not owned (ownership is passed to the SyncManager).
+ MockUnrecoverableErrorHandler* mock_unrecoverable_error_handler_;
};
TEST_F(SyncManagerTest, GetAllNodesForTypeTest) {
@@ -2058,6 +2069,93 @@ TEST_F(SyncManagerTest, UpdatePasswordReencryptEverything) {
EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, client_tag));
}
+// Test that attempting to start up with corrupted password data triggers
+// an unrecoverable error (rather than crashing).
+TEST_F(SyncManagerTest, ReencryptEverythingWithUnrecoverableErrorPasswords) {
+ const char kClientTag[] = "client_tag";
+
+ EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION));
+ sync_pb::EntitySpecifics entity_specifics;
+ {
+ // Create a synced bookmark with undecryptable data.
+ ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
+
+ Cryptographer other_cryptographer(&encryptor_);
+ KeyParams fake_params = {"localhost", "dummy", "fake_key"};
+ other_cryptographer.AddKey(fake_params);
+ sync_pb::PasswordSpecificsData data;
+ data.set_password_value("secret");
+ other_cryptographer.Encrypt(
+ data,
+ entity_specifics.mutable_password()->mutable_encrypted());
+
+ // Set up the real cryptographer with a different key.
+ KeyParams real_params = {"localhost", "username", "real_key"};
+ trans.GetCryptographer()->AddKey(real_params);
+ }
+ MakeServerNode(sync_manager_.GetUserShare(), PASSWORDS, kClientTag,
+ syncable::GenerateSyncableHash(PASSWORDS,
+ kClientTag),
+ entity_specifics);
+ EXPECT_FALSE(ResetUnsyncedEntry(PASSWORDS, kClientTag));
+
+ // Force a re-encrypt everything. Should trigger an unrecoverable error due
+ // to being unable to decrypt the data that was previously applied.
+ testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
+ EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
+ EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
+ EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, false));
+ EXPECT_FALSE(HasUnrecoverableError());
+ sync_manager_.GetEncryptionHandler()->Init();
+ PumpLoop();
+ EXPECT_TRUE(HasUnrecoverableError());
+}
+
+// Test that attempting to start up with corrupted bookmark data triggers
+// an unrecoverable error (rather than crashing).
+TEST_F(SyncManagerTest, ReencryptEverythingWithUnrecoverableErrorBookmarks) {
+ const char kClientTag[] = "client_tag";
+ EXPECT_CALL(encryption_observer_,
+ OnEncryptedTypesChanged(
+ HasModelTypes(EncryptableUserTypes()), true));
+ EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, FULL_ENCRYPTION));
+ sync_pb::EntitySpecifics entity_specifics;
+ {
+ // Create a synced bookmark with undecryptable data.
+ ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare());
+
+ Cryptographer other_cryptographer(&encryptor_);
+ KeyParams fake_params = {"localhost", "dummy", "fake_key"};
+ other_cryptographer.AddKey(fake_params);
+ sync_pb::EntitySpecifics bm_specifics;
+ bm_specifics.mutable_bookmark()->set_title("title");
+ bm_specifics.mutable_bookmark()->set_url("url");
+ sync_pb::EncryptedData encrypted;
+ other_cryptographer.Encrypt(bm_specifics, &encrypted);
+ entity_specifics.mutable_encrypted()->CopyFrom(encrypted);
+
+ // Set up the real cryptographer with a different key.
+ KeyParams real_params = {"localhost", "username", "real_key"};
+ trans.GetCryptographer()->AddKey(real_params);
+ }
+ MakeServerNode(sync_manager_.GetUserShare(), BOOKMARKS, kClientTag,
+ syncable::GenerateSyncableHash(BOOKMARKS,
+ kClientTag),
+ entity_specifics);
+ EXPECT_FALSE(ResetUnsyncedEntry(BOOKMARKS, kClientTag));
+
+ // Force a re-encrypt everything. Should trigger an unrecoverable error due
+ // to being unable to decrypt the data that was previously applied.
+ testing::Mock::VerifyAndClearExpectations(&encryption_observer_);
+ EXPECT_CALL(encryption_observer_, OnEncryptionComplete());
+ EXPECT_CALL(encryption_observer_, OnCryptographerStateChanged(_));
+ EXPECT_CALL(encryption_observer_, OnEncryptedTypesChanged(_, true));
+ EXPECT_FALSE(HasUnrecoverableError());
+ sync_manager_.GetEncryptionHandler()->Init();
+ PumpLoop();
+ EXPECT_TRUE(HasUnrecoverableError());
+}
+
// Verify SetTitle(..) doesn't unnecessarily set IS_UNSYNCED for bookmarks
// when we write the same data, but does set it when we write new data.
TEST_F(SyncManagerTest, SetBookmarkTitle) {
« no previous file with comments | « sync/internal_api/sync_encryption_handler_impl.cc ('k') | sync/internal_api/syncapi_internal.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698