Fix keyword table access in the same way credit_card table was just fixed. Th...

chrome/browser/webdata/web_database.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/webdata/web_database.h"
 
 #include <algorithm>
 #include <limits>
 #include <set>
 #include <string>
@@ skipping 429 matching lines @@
 
   // Version check.
   if (!meta_table_.Init(&db_, kCurrentVersionNumber, kCompatibleVersionNumber))
     return sql::INIT_FAILURE;
   if (meta_table_.GetCompatibleVersionNumber() > kCurrentVersionNumber) {
     LOG(WARNING) << "Web database is too new.";
     return sql::INIT_TOO_NEW;
   }
 
   // Initialize the tables.
-  bool credit_card_table_created = false;
   if (!InitKeywordsTable() || !InitLoginsTable() || !InitWebAppIconsTable() ||
       !InitWebAppsTable() || !InitAutofillTable() ||
       !InitAutofillDatesTable() || !InitAutoFillProfilesTable() ||
-      !InitCreditCardsTable(&credit_card_table_created) ||
-      !InitTokenServiceTable()) {
+      !InitCreditCardsTable() || !InitTokenServiceTable()) {
     LOG(WARNING) << "Unable to initialize the web database.";
     return sql::INIT_FAILURE;
   }
 
   // If the file on disk is an older database version, bring it up to date.
-  MigrateOldVersionsAsNeeded(credit_card_table_created);
+  MigrateOldVersionsAsNeeded();
 
   return transaction.Commit() ? sql::INIT_OK : sql::INIT_FAILURE;
 }
 
 bool WebDatabase::SetWebAppImage(const GURL& url, const SkBitmap& image) {
   // Don't bother with a cached statement since this will be a relatively
   // infrequent operation.
   sql::Statement s(db_.GetUniqueStatement(
       "INSERT OR REPLACE INTO web_app_icons "
       "(url, width, height, image) VALUES (?, ?, ?, ?)"));
@@ skipping 283 matching lines @@
     }
     if (!db_.Execute("CREATE INDEX autofill_profiles_label_index "
                      "ON autofill_profiles (label)")) {
       NOTREACHED();
       return false;
     }
   }
   return true;
 }
 
-bool WebDatabase::InitCreditCardsTable(bool* table_was_created) {
+bool WebDatabase::InitCreditCardsTable() {
   if (!db_.DoesTableExist("credit_cards")) {
     if (!db_.Execute("CREATE TABLE credit_cards ( "
                      "label VARCHAR, "
                      "unique_id INTEGER PRIMARY KEY, "
                      "name_on_card VARCHAR, "
                      "type VARCHAR, "
                      "card_number VARCHAR, "
                      "expiration_month INTEGER, "
                      "expiration_year INTEGER, "
                      "verification_code VARCHAR, "
                      "billing_address VARCHAR, "
                      "shipping_address VARCHAR, "
                      "card_number_encrypted BLOB, "
                      "verification_code_encrypted BLOB)")) {
       NOTREACHED();
       return false;
     }
-    *table_was_created = true;
     if (!db_.Execute("CREATE INDEX credit_cards_label_index "
                      "ON credit_cards (label)")) {
       NOTREACHED();
       return false;
     }
   }
   return true;
 }
 
 bool WebDatabase::InitWebAppIconsTable() {
@@ skipping 1073 matching lines @@
     return false;
   }
   s.BindInt64(0, pair_id);
   if (s.Run()) {
     return RemoveFormElementForTimeRange(pair_id, base::Time(), base::Time(),
                                          NULL);
   }
   return false;
 }
 
-void WebDatabase::MigrateOldVersionsAsNeeded(bool credit_card_table_created) {
+void WebDatabase::MigrateOldVersionsAsNeeded(){
   // Migrate if necessary.
   int current_version = meta_table_.GetVersionNumber();
   switch (current_version) {
     // Versions 1 - 19 are unhandled.  Version numbers greater than
     // kCurrentVersionNumber should have already been weeded out by the caller.
     default:
       // When the version is too old, we just try to continue anyway.  There
       // should not be a released product that makes a database too old for us
       // to handle.
       LOG(WARNING) << "Web database version " << current_version <<
@@ skipping 17 matching lines @@
       if (!ClearAutofillEmptyValueElements()) {
         NOTREACHED() << "Failed to clean-up autofill DB.";
       }
       meta_table_.SetVersionNumber(22);
       // No change in the compatibility version number.
 
       // FALL THROUGH
 
     case 22:
       // Add the card_number_encrypted column if credit card table was not
-      // created in this build.
-      if (!credit_card_table_created) {
-        if (!db_.Execute("ALTER TABLE credit_cards ADD COLUMN "
-                         "card_number_encrypted BLOB DEFAULT NULL")) {
-            NOTREACHED();
-            LOG(WARNING) << "Unable to update web database to version 23.";
-            return;
-        }
-        if (!db_.Execute("ALTER TABLE credit_cards ADD COLUMN "
-                         "verification_code_encrypted BLOB DEFAULT NULL")) {
-            NOTREACHED();
-            LOG(WARNING) << "Unable to update web database to version 23.";
-            return;
-        }
+      // created in this build (otherwise the column already exists).
+      // WARNING: Do not change the order of the execution of the SQL
+      // statements in this case!  Profile corruption and data migration
+      // issues WILL OCCUR. (see http://crbug.com/10913)
+      //
+      // The problem is that if a user has a profile which was created before
+      // r37036, when the credit_cards table was added, and then failed to
+      // update this profile between the credit card addition and the addition
+      // of the "encrypted" columns (44963), the next data migration will put
+      // the user's profile in an incoherent state: The user will update from
+      // a data profile set to be earlier than 22, and therefore pass through
+      // this update case.  But because the user did not have a credit_cards
+      // table before starting Chrome, it will have just been initialized
+      // above, and so already have these columns -- and thus this data
+      // update step will have failed.
+      //
+      // The false assumption in this case is that at this step in the
+      // migration, the user has a credit card table, and that this
+      // table does not include encrypted columns!
+      // Because this case does not roll back the complete set of SQL
+      // transactions properly in case of failure (that is, it does not
+      // roll back the table initialization done above), the incoherent
+      // profile will now see itself as being at version 22 -- but include a
+      // fully initialized credit_cards table.  Every time Chrome runs, it
+      // will try to update the web database and fail at this step, unless
+      // we allow for the faulty assumption described above by checking for
+      // the existence of the columns only AFTER we've executed the commands
+      // to add them.
+      if (!db_.Execute("ALTER TABLE credit_cards ADD COLUMN "
+                       "card_number_encrypted BLOB DEFAULT NULL")) {
+        LOG(WARNING) << "Could not add card_number_encrypted to "
+                        "credit_cards table.";
+      }
+      if (!db_.Execute("ALTER TABLE credit_cards ADD COLUMN "
+                       "verification_code_encrypted BLOB DEFAULT NULL")) {
+        LOG(WARNING) << "Could not add verification_code_encrypted to "
+                        "credit_cards table.";
+      }
+      if (!db_.Execute(
+              "SELECT card_number_encrypted FROM credit_cards limit 1") ||
+          !db_.Execute(
+              "SELECT verification_code_encrypted FROM credit_cards limit 1")) {
+        NOTREACHED();
+        LOG(WARNING) << "Unable to update web database to version 23.";
+        return;
       }
       meta_table_.SetVersionNumber(23);
-
       // FALL THROUGH
 
     case 23: {
       // One-time cleanup for Chromium bug 38364.  In the presence of
       // multi-byte UTF-8 characters, that bug could cause AutoFill strings
       // to grow larger and more corrupt with each save.  The cleanup removes
       // any row with a string field larger than a reasonable size.  The string
       // fields examined here are precisely the ones that were subject to
       // corruption by the original bug.
       const std::string autofill_is_too_big =
@@ skipping 41 matching lines @@
         LOG(WARNING) << "Unable to update web database to version 24.";
         return;
       }
 
       meta_table_.SetVersionNumber(24);
 
       // FALL THROUGH
     }
 
     case 24:
-      // Add the logo_id column.
+      // Add the logo_id column if keyword table was not created in this build.
       if (!db_.Execute("ALTER TABLE keywords ADD COLUMN logo_id "
                        "INTEGER DEFAULT 0")) {
         NOTREACHED();
         LOG(WARNING) << "Unable to update web database to version 25.";
         return;
       }
       meta_table_.SetVersionNumber(25);
       meta_table_.SetCompatibleVersionNumber(
           std::min(25, kCompatibleVersionNumber));
       // FALL THROUGH
 
     // Add successive versions here.  Each should set the version number and
     // compatible version number as appropriate, then fall through to the next
     // case.
 
     case kCurrentVersionNumber:
       // No migration needed.
       return;
   }
 }