sqlite_utils: don't dereference an empty vector.

user_settings: Add unit test intended to exercise the above path.
In practice though these tests don't hit the bug, since
EncryptData("") results in non-empty ciphertext.

bonus: don't leak a sqlite handle in one of the other sync unit tests.

TEST=none
BUG=40217

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

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

3 files changed, 36 insertions, 6 deletions

diff --git a/chrome/browser/sync/util/data_encryption.cc b/chrome/browser/sync/util/data_encryption.cc
index b835147..1232447 100644
--- a/chrome/browser/sync/util/data_encryption.cc
+++ b/chrome/browser/sync/util/data_encryption.cc
@@ -17,9 +17,10 @@ using std::string;
 using std::vector;
 
 vector<uint8> EncryptData(const string& data) {
-  DATA_BLOB unencrypted_data, encrypted_data;
+  DATA_BLOB unencrypted_data = { 0 };
   unencrypted_data.pbData = (BYTE*)(data.data());
   unencrypted_data.cbData = data.size();
+  DATA_BLOB encrypted_data = { 0 };
 
   if (!CryptProtectData(&unencrypted_data, L"", NULL, NULL, NULL, 0,
                         &encrypted_data))
diff --git a/chrome/browser/sync/util/user_settings_unittest.cc b/chrome/browser/sync/util/user_settings_unittest.cc
index 073c29c..a39f987 100644
--- a/chrome/browser/sync/util/user_settings_unittest.cc
+++ b/chrome/browser/sync/util/user_settings_unittest.cc
@@ -40,6 +40,8 @@ class UserSettingsTest : public testing::Test {
         destination_directory.Append(FilePath(kV10UserSettingsDB));
     ASSERT_EQ(SQLITE_OK, sqlite_utils::OpenSqliteDb(v10_user_setting_db_path_,
                                                     &primer_handle));
+    sqlite_utils::scoped_sqlite_db_ptr db(primer_handle);
+
     old_style_sync_data_path_ =
         destination_directory.Append(FilePath(kOldStyleSyncDataDB));
 
@@ -82,7 +84,6 @@ class UserSettingsTest : public testing::Test {
         LOG(FATAL) << query << "\n" << sqlite3_errmsg(primer_handle);
       }
     }
-    sqlite3_close(primer_handle);
   }
 
    // Creates and populates the V11 database file within
@@ -93,6 +94,7 @@ class UserSettingsTest : public testing::Test {
         destination_directory.Append(FilePath(kV11UserSettingsDB));
     ASSERT_EQ(SQLITE_OK, sqlite_utils::OpenSqliteDb(v11_user_setting_db_path_,
                                                     &primer_handle));
+    sqlite_utils::scoped_sqlite_db_ptr db(primer_handle);
 
     // Create settings table.
     ExecOrDie(primer_handle, "CREATE TABLE settings"
@@ -124,8 +126,6 @@ class UserSettingsTest : public testing::Test {
         LOG(FATAL) << query << "\n" << sqlite3_errmsg(primer_handle);
       }
     }
-
-    sqlite3_close(primer_handle);
   }
 
   const std::string& sync_data() const { return sync_data_; }
@@ -164,6 +164,7 @@ TEST_F(UserSettingsTest, MigrateFromV10ToV11) {
   sqlite3* handle = NULL;
   ASSERT_EQ(SQLITE_OK, sqlite_utils::OpenSqliteDb(v10_user_setting_db_path(),
                                                   &handle));
+  sqlite_utils::scoped_sqlite_db_ptr db(handle);
 
   // Note that we don't use ScopedStatement to avoid closing the sqlite handle
   // before finalizing the statement.
@@ -183,7 +184,6 @@ TEST_F(UserSettingsTest, MigrateFromV10ToV11) {
   std::string contents;
   ASSERT_TRUE(file_util::ReadFileToString(new_style_path, &contents));
   EXPECT_TRUE(sync_data() == contents);
-  sqlite3_close(handle);
 }
 
 TEST_F(UserSettingsTest, MigrateFromV11ToV12) {
@@ -197,6 +197,7 @@ TEST_F(UserSettingsTest, MigrateFromV11ToV12) {
   sqlite3* handle = NULL;
   ASSERT_EQ(SQLITE_OK, sqlite_utils::OpenSqliteDb(v11_user_setting_db_path(),
                                                   &handle));
+  sqlite_utils::scoped_sqlite_db_ptr db(handle);
 
   {
     SQLStatement version_query;
@@ -222,3 +223,31 @@ TEST_F(UserSettingsTest, APEncode) {
   const string decoded = APDecode(encoded);
   ASSERT_EQ(test, decoded);
 }
+
+TEST_F(UserSettingsTest, PersistEmptyToken) {
+  ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  UserSettings settings;
+  settings.Init(temp_dir.path().AppendASCII("UserSettings.sqlite3"));
+  settings.SetAuthTokenForService("username", "service", "");
+  std::string username;
+  std::string token;
+  ASSERT_TRUE(settings.GetLastUserAndServiceToken("service", &username,
+      &token));
+  EXPECT_EQ("", token);
+  EXPECT_EQ("username", username);
+}
+
+TEST_F(UserSettingsTest, PersistNonEmptyToken) {
+  ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+  UserSettings settings;
+  settings.Init(temp_dir.path().AppendASCII("UserSettings.sqlite3"));
+  settings.SetAuthTokenForService("username", "service", "012345beefbeef");
+  std::string username;
+  std::string token;
+  ASSERT_TRUE(settings.GetLastUserAndServiceToken("service", &username,
+      &token));
+  EXPECT_EQ("012345beefbeef", token);
+  EXPECT_EQ("username", username);
+}
diff --git a/chrome/common/sqlite_utils.cc b/chrome/common/sqlite_utils.cc
index 8e108a7..b99b8f2 100644
--- a/chrome/common/sqlite_utils.cc
+++ b/chrome/common/sqlite_utils.cc
@@ -323,7 +323,7 @@ int SQLStatement::bind_parameter_count() {
 
 int SQLStatement::bind_blob(int index, std::vector<unsigned char>* blob) {
   if (blob) {
-    const void* value = &(*blob)[0];
+    const void* value = blob->empty() ? NULL : &(*blob)[0];
     int len = static_cast<int>(blob->size());
     return bind_blob(index, value, len);
   } else {