[epiphany/mcatanzaro/gsb-storage-take-two] gsb-storage: Avoid crashes after database is recreated



commit 51926d42408462128e09b83bb51aa0bb63840ffa
Author: Michael Catanzaro <mcatanzaro igalia com>
Date:   Wed May 8 12:46:40 2019 -0500

    gsb-storage: Avoid crashes after database is recreated
    
    Unfortunately commit 2475b4f0 has introduced some crashes after the GSB
    database needs to be recreated due to database corruption, which is
    still happening in practice. The entire file now needs to be prepared
    for a previously-functional database to become inoperable at any time.

 lib/safe-browsing/ephy-gsb-storage.c | 99 ++++++++++++++++++++++++++----------
 1 file changed, 71 insertions(+), 28 deletions(-)
---
diff --git a/lib/safe-browsing/ephy-gsb-storage.c b/lib/safe-browsing/ephy-gsb-storage.c
index 29c938a15..44e3aa683 100644
--- a/lib/safe-browsing/ephy-gsb-storage.c
+++ b/lib/safe-browsing/ephy-gsb-storage.c
@@ -124,7 +124,9 @@ ephy_gsb_storage_start_transaction (EphyGSBStorage *self)
   GError *error = NULL;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
+
+  if (!self->is_operable)
+    return;
 
   ephy_sqlite_connection_begin_transaction (self->db, &error);
   if (error) {
@@ -139,7 +141,9 @@ ephy_gsb_storage_end_transaction (EphyGSBStorage *self)
   GError *error = NULL;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
+
+  if (!self->is_operable)
+    return;
 
   ephy_sqlite_connection_commit_transaction (self->db, &error);
   if (error) {
@@ -382,11 +386,12 @@ static void
 ephy_gsb_storage_clear_db (EphyGSBStorage *self)
 {
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (EPHY_IS_SQLITE_CONNECTION (self->db));
 
-  ephy_sqlite_connection_close (self->db);
-  ephy_sqlite_connection_delete_database (self->db);
-  g_clear_object (&self->db);
+  if (self->db) {
+    ephy_sqlite_connection_close (self->db);
+    ephy_sqlite_connection_delete_database (self->db);
+    g_clear_object (&self->db);
+  }
 }
 
 static gboolean
@@ -408,6 +413,8 @@ ephy_gsb_storage_init_db (EphyGSBStorage *self)
   if (!success)
     ephy_gsb_storage_clear_db (self);
 
+  self->is_operable = success;
+
   return success;
 }
 
@@ -498,11 +505,9 @@ ephy_gsb_storage_constructed (GObject *object)
     success = ephy_gsb_storage_open_db (self);
     if (success && !ephy_gsb_storage_check_schema_version (self)) {
       LOG ("GSB database schema incompatibility, recreating database...");
-      success = ephy_gsb_storage_recreate_db (self);
+      ephy_gsb_storage_recreate_db (self);
     }
   }
-
-  self->is_operable = success;
 }
 
 static void
@@ -627,9 +632,11 @@ ephy_gsb_storage_set_metadata (EphyGSBStorage *self,
   const char *sql;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (key);
 
+  if (!self->is_operable)
+    return;
+
   sql = "UPDATE metadata SET value=? WHERE key=?";
   statement = ephy_sqlite_connection_create_statement (self->db, sql, &error);
   if (error) {
@@ -686,7 +693,9 @@ ephy_gsb_storage_get_threat_lists (EphyGSBStorage *self)
   const char *sql;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
+
+  if (!self->is_operable)
+    return NULL;
 
   sql = "SELECT threat_type, platform_type, threat_entry_type, client_state FROM threats";
   statement = ephy_sqlite_connection_create_statement (self->db, sql, &error);
@@ -742,9 +751,11 @@ ephy_gsb_storage_compute_checksum (EphyGSBStorage    *self,
   gsize digest_len = GSB_HASH_SIZE;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (list);
 
+  if (!self->is_operable)
+    return NULL;
+
   sql = "SELECT value FROM hash_prefix WHERE "
         "threat_type=? AND platform_type=? AND threat_entry_type=? "
         "ORDER BY value";
@@ -807,9 +818,11 @@ ephy_gsb_storage_update_client_state (EphyGSBStorage    *self,
   gboolean success;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (list);
 
+  if (!self->is_operable)
+    return;
+
   if (clear) {
     sql = "UPDATE threats SET client_state=NULL "
           "WHERE threat_type=? AND platform_type=? AND threat_entry_type=?";
@@ -861,9 +874,11 @@ ephy_gsb_storage_clear_hash_prefixes (EphyGSBStorage    *self,
   const char *sql;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (list);
 
+  if (!self->is_operable)
+    return;
+
   sql = "DELETE FROM hash_prefix WHERE "
         "threat_type=? AND platform_type=? AND threat_entry_type=?";
   statement = ephy_sqlite_connection_create_statement (self->db, sql, &error);
@@ -901,10 +916,12 @@ ephy_gsb_storage_get_hash_prefixes_to_delete (EphyGSBStorage    *self,
   guint index = 0;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (list);
   g_assert (indices);
 
+  if (!self->is_operable)
+    return NULL;
+
   *num_prefixes = 0;
 
   sql = "SELECT value FROM hash_prefix WHERE "
@@ -952,7 +969,9 @@ ephy_gsb_storage_make_delete_hash_prefix_statement (EphyGSBStorage *self,
   GString *sql;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
+
+  if (!self->is_operable)
+    return NULL;
 
   sql = g_string_new ("DELETE FROM hash_prefix WHERE "
                       "threat_type=? AND platform_type=? and threat_entry_type=? "
@@ -985,10 +1004,12 @@ ephy_gsb_storage_delete_hash_prefixes_batch (EphyGSBStorage      *self,
   gboolean free_statement = TRUE;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (list);
   g_assert (prefixes);
 
+  if (!self->is_operable)
+    return NULL;
+
   if (stmt) {
     statement = stmt;
     ephy_sqlite_statement_reset (statement);
@@ -1042,10 +1063,12 @@ ephy_gsb_storage_delete_hash_prefixes_internal (EphyGSBStorage    *self,
   gsize num_prefixes;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (list);
   g_assert (indices);
 
+  if (!self->is_operable)
+    return;
+
   LOG ("Deleting %lu hash prefixes...", num_indices);
 
   /* Move indices from the array to a hash table set. */
@@ -1105,10 +1128,12 @@ ephy_gsb_storage_delete_hash_prefixes (EphyGSBStorage    *self,
   gsize num_indices;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (list);
   g_assert (tes);
 
+  if (!self->is_operable)
+    return;
+
   compression = json_object_get_string_member (tes, "compressionType");
   if (!g_strcmp0 (compression, GSB_COMPRESSION_TYPE_RICE)) {
     rice_indices = json_object_get_object_member (tes, "riceIndices");
@@ -1137,7 +1162,9 @@ ephy_gsb_storage_make_insert_hash_prefix_statement (EphyGSBStorage *self,
   GString *sql;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
+
+  if (!self->is_operable)
+    return NULL;
 
   sql = g_string_new ("INSERT INTO hash_prefix "
                       "(cue, value, threat_type, platform_type, threat_entry_type) VALUES ");
@@ -1172,10 +1199,12 @@ ephy_gsb_storage_insert_hash_prefixes_batch (EphyGSBStorage      *self,
   gboolean free_statement = TRUE;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (list);
   g_assert (prefixes);
 
+  if (!self->is_operable)
+    return;
+
   if (stmt) {
     statement = stmt;
     ephy_sqlite_statement_reset (statement);
@@ -1219,10 +1248,12 @@ ephy_gsb_storage_insert_hash_prefixes_internal (EphyGSBStorage    *self,
   gsize num_batches;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (list);
   g_assert (prefixes);
 
+  if (!self->is_operable)
+    return;
+
   LOG ("Inserting %lu hash prefixes of size %ld...", num_prefixes, prefix_len);
 
   ephy_gsb_storage_start_transaction (self);
@@ -1280,10 +1311,12 @@ ephy_gsb_storage_insert_hash_prefixes (EphyGSBStorage    *self,
   gsize num_prefixes;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (list);
   g_assert (tes);
 
+  if (!self->is_operable)
+    return;
+
   compression = json_object_get_string_member (tes, "compressionType");
   if (!g_strcmp0 (compression, GSB_COMPRESSION_TYPE_RICE)) {
     rice_hashes = json_object_get_object_member (tes, "riceHashes");
@@ -1335,9 +1368,11 @@ ephy_gsb_storage_lookup_hash_prefixes (EphyGSBStorage *self,
   guint id = 0;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (cues);
 
+  if (!self->is_operable)
+    return NULL;
+
   sql = g_string_new ("SELECT value, negative_expires_at <= (CAST(strftime('%s', 'now') AS INT)) "
                       "FROM hash_prefix WHERE cue IN (");
   for (GList *l = cues; l && l->data; l = l->next)
@@ -1412,9 +1447,11 @@ ephy_gsb_storage_lookup_full_hashes (EphyGSBStorage *self,
   guint id = 0;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (hashes);
 
+  if (!self->is_operable)
+    return NULL;
+
   sql = g_string_new ("SELECT value, threat_type, platform_type, threat_entry_type, "
                       "expires_at <= (CAST(strftime('%s', 'now') AS INT)) "
                       "FROM hash_full WHERE value IN (");
@@ -1494,10 +1531,12 @@ ephy_gsb_storage_insert_full_hash (EphyGSBStorage    *self,
   const char *sql;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (list);
   g_assert (hash);
 
+  if (!self->is_operable)
+    return;
+
   LOG ("Inserting full hash with duration %ld for list %s/%s/%s",
        duration, list->threat_type, list->platform_type, list->threat_entry_type);
 
@@ -1576,7 +1615,9 @@ ephy_gsb_storage_delete_old_full_hashes (EphyGSBStorage *self)
   const char *sql;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
+
+  if (!self->is_operable)
+    return;
 
   LOG ("Deleting full hashes expired for more than %d seconds", EXPIRATION_THRESHOLD);
 
@@ -1625,9 +1666,11 @@ ephy_gsb_storage_update_hash_prefix_expiration (EphyGSBStorage *self,
   const char *sql;
 
   g_assert (EPHY_IS_GSB_STORAGE (self));
-  g_assert (self->is_operable);
   g_assert (prefix);
 
+  if (!self->is_operable)
+    return;
+
   sql = "UPDATE hash_prefix "
         "SET negative_expires_at=(CAST(strftime('%s', 'now') AS INT)) + ? "
         "WHERE value=?";


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]