fix: harden table initialization against stale config and error cleanup

- Skip stale table settings for dropped tables during init by checking
  database_table_exists() before creating triggers
- Add col_name to cloudsync_table_settings PRIMARY KEY in SQLite to
  match PostgreSQL and prevent REPLACE INTO from silently overwriting
  column-level settings
- Zero-initialize column arrays (col_name, col_id, col_merge_stmt,
  col_value_stmt) to prevent SIGSEGV when error cleanup calls
  databasevm_finalize on uninitialized pointers
- Free column arrays in table_free unconditionally so they are not
  leaked when table_add_to_context_cb fails before incrementing ncols

Author: andinux

src/cloudsync.c

@@ -720,37 +720,35 @@ void table_free (cloudsync_table_context *table) {
     DEBUG_DBFUNCTION("table_free %s", (table) ? (table->name) : "NULL");
     if (!table) return;
 
-    if (table->ncols > 0) {
-        if (table->col_name) {
-            for (int i=0; i<table->ncols; ++i) {
-                cloudsync_memory_free(table->col_name[i]);
-            }
-            cloudsync_memory_free(table->col_name);
-        }
-        if (table->col_merge_stmt) {
-            for (int i=0; i<table->ncols; ++i) {
-                databasevm_finalize(table->col_merge_stmt[i]);
-            }
-            cloudsync_memory_free(table->col_merge_stmt);
-        }
-        if (table->col_value_stmt) {
-            for (int i=0; i<table->ncols; ++i) {
-                databasevm_finalize(table->col_value_stmt[i]);
-            }
-            cloudsync_memory_free(table->col_value_stmt);
+    if (table->col_name) {
+        for (int i=0; i<table->ncols; ++i) {
+            cloudsync_memory_free(table->col_name[i]);
         }
-        if (table->col_id) {
-            cloudsync_memory_free(table->col_id);
+        cloudsync_memory_free(table->col_name);
+    }
+    if (table->col_merge_stmt) {
+        for (int i=0; i<table->ncols; ++i) {
+            databasevm_finalize(table->col_merge_stmt[i]);
         }
-        if (table->col_algo) {
-            cloudsync_memory_free(table->col_algo);
+        cloudsync_memory_free(table->col_merge_stmt);
+    }
+    if (table->col_value_stmt) {
+        for (int i=0; i<table->ncols; ++i) {
+            databasevm_finalize(table->col_value_stmt[i]);
         }
-        if (table->col_delimiter) {
-            for (int i=0; i<table->ncols; ++i) {
-                if (table->col_delimiter[i]) cloudsync_memory_free(table->col_delimiter[i]);
-            }
-            cloudsync_memory_free(table->col_delimiter);
+        cloudsync_memory_free(table->col_value_stmt);
+    }
+    if (table->col_id) {
+        cloudsync_memory_free(table->col_id);
+    }
+    if (table->col_algo) {
+        cloudsync_memory_free(table->col_algo);
+    }
+    if (table->col_delimiter) {
+        for (int i=0; i<table->ncols; ++i) {
+            if (table->col_delimiter[i]) cloudsync_memory_free(table->col_delimiter[i]);
         }
+        cloudsync_memory_free(table->col_delimiter);
     }
 
     if (table->block_value_read_stmt) databasevm_finalize(table->block_value_read_stmt);
@@ -1079,16 +1077,16 @@ bool table_add_to_context (cloudsync_context *data, table_algo algo, const char
 
     // a table with only pk(s) is totally legal
     if (ncols > 0) {
-        table->col_name = (char **)cloudsync_memory_alloc((uint64_t)(sizeof(char *) * ncols));
+        table->col_name = (char **)cloudsync_memory_zeroalloc((uint64_t)(sizeof(char *) * ncols));
         if (!table->col_name) goto abort_add_table;
-        
-        table->col_id = (int *)cloudsync_memory_alloc((uint64_t)(sizeof(int) * ncols));
+
+        table->col_id = (int *)cloudsync_memory_zeroalloc((uint64_t)(sizeof(int) * ncols));
         if (!table->col_id) goto abort_add_table;
-        
-        table->col_merge_stmt = (dbvm_t **)cloudsync_memory_alloc((uint64_t)(sizeof(void *) * ncols));
+
+        table->col_merge_stmt = (dbvm_t **)cloudsync_memory_zeroalloc((uint64_t)(sizeof(void *) * ncols));
         if (!table->col_merge_stmt) goto abort_add_table;
-        
-        table->col_value_stmt = (dbvm_t **)cloudsync_memory_alloc((uint64_t)(sizeof(void *) * ncols));
+
+        table->col_value_stmt = (dbvm_t **)cloudsync_memory_zeroalloc((uint64_t)(sizeof(void *) * ncols));
         if (!table->col_value_stmt) goto abort_add_table;
 
         table->col_algo = (col_algo_t *)cloudsync_memory_zeroalloc((uint64_t)(sizeof(col_algo_t) * ncols));

src/cloudsync.h

@@ -18,7 +18,7 @@
 extern "C" {
 #endif
 
-#define CLOUDSYNC_VERSION                       "1.0.6"
+#define CLOUDSYNC_VERSION                       "1.0.7"
 #define CLOUDSYNC_MAX_TABLENAME_LEN             512
 
 #define CLOUDSYNC_VALUE_NOTSET                  -1

src/dbutils.c

@@ -363,6 +363,11 @@ int dbutils_settings_table_load_callback (void *xdata, int ncols, char **values,
 
         // Table-level algo setting (col_name == "*")
         if (strcmp(key, "algo") == 0 && col_name && strcmp(col_name, "*") == 0) {
+            // Skip stale settings for tables that no longer exist
+            if (!database_table_exists(data, table_name, cloudsync_schema(data))) {
+                DEBUG_SETTINGS("skipping stale settings for dropped table: %s", table_name);
+                continue;
+            }
             table_algo algo = cloudsync_algo_from_name(value);
             char fbuf[2048];
             int frc = dbutils_table_settings_get_value(data, table_name, "*", "filter", fbuf, sizeof(fbuf));

src/sqlite/sql_sqlite.c

@@ -56,7 +56,7 @@ const char * const SQL_INSERT_SITE_ID_ROWID =
     "INSERT INTO cloudsync_site_id (rowid, site_id) VALUES (?, ?);";
 
 const char * const SQL_CREATE_TABLE_SETTINGS_TABLE =
-    "CREATE TABLE IF NOT EXISTS cloudsync_table_settings (tbl_name TEXT NOT NULL COLLATE NOCASE, col_name TEXT NOT NULL COLLATE NOCASE, key TEXT, value TEXT, PRIMARY KEY(tbl_name,key));";
+    "CREATE TABLE IF NOT EXISTS cloudsync_table_settings (tbl_name TEXT NOT NULL COLLATE NOCASE, col_name TEXT NOT NULL COLLATE NOCASE, key TEXT, value TEXT, PRIMARY KEY(tbl_name,col_name,key));";
 
 const char * const SQL_CREATE_SCHEMA_VERSIONS_TABLE =
     "CREATE TABLE IF NOT EXISTS cloudsync_schema_versions (hash INTEGER PRIMARY KEY, seq INTEGER NOT NULL)";

test/unit.c

@@ -2330,6 +2330,155 @@ bool do_test_error_handling(void) {
     return result;
 }
 
+// Test that stale table settings don't crash on reload.
+// Exercises the error cleanup path in table_add_to_context_cb:
+// if a table is dropped without cloudsync_cleanup(), its settings
+// persist as orphans. On re-init the zero-initialized column arrays
+// must prevent databasevm_finalize from being called on garbage pointers.
+bool do_test_stale_table_settings(bool cleanup_databases) {
+    bool result = false;
+    char dbpath[256];
+    time_t timestamp = time(NULL);
+
+    #ifdef __ANDROID__
+    snprintf(dbpath, sizeof(dbpath), "%s/cloudsync-test-stale-%ld.sqlite", ".", timestamp);
+    #else
+    snprintf(dbpath, sizeof(dbpath), "%s/cloudsync-test-stale-%ld.sqlite", getenv("HOME"), timestamp);
+    #endif
+
+    // Phase 1: create database, table, and init cloudsync
+    sqlite3 *db = NULL;
+    int rc = sqlite3_open(dbpath, &db);
+    if (rc != SQLITE_OK) return false;
+    sqlite3_exec(db, "PRAGMA journal_mode=WAL;", NULL, NULL, NULL);
+    sqlite3_cloudsync_init(db, NULL, NULL);
+
+    rc = sqlite3_exec(db, "CREATE TABLE cloud (id TEXT PRIMARY KEY NOT NULL, value TEXT, extra INTEGER);", NULL, NULL, NULL);
+    if (rc != SQLITE_OK) goto cleanup;
+
+    rc = sqlite3_exec(db, "SELECT cloudsync_init('cloud');", NULL, NULL, NULL);
+    if (rc != SQLITE_OK) goto cleanup;
+
+    // Phase 2: drop the table WITHOUT calling cloudsync_cleanup
+    // This leaves stale entries in cloudsync_table_settings
+    sqlite3_exec(db, "SELECT cloudsync_terminate();", NULL, NULL, NULL);
+    sqlite3_close(db);
+    db = NULL;
+
+    // Reopen and drop the table directly (bypassing cloudsync)
+    rc = sqlite3_open(dbpath, &db);
+    if (rc != SQLITE_OK) goto cleanup;
+    rc = sqlite3_exec(db, "DROP TABLE IF EXISTS cloud;", NULL, NULL, NULL);
+    if (rc != SQLITE_OK) goto cleanup;
+    sqlite3_close(db);
+    db = NULL;
+
+    // Phase 3: reopen the database — sqlite3_cloudsync_init will load
+    // stale settings and try to re-create triggers for the dropped table.
+    // Without the fix this would crash (SIGSEGV) or return an error.
+    rc = sqlite3_open(dbpath, &db);
+    if (rc != SQLITE_OK) goto cleanup;
+    rc = sqlite3_cloudsync_init(db, NULL, NULL);
+    if (rc != SQLITE_OK) goto cleanup;
+
+    // If we reach here without crashing, the fix works
+    result = true;
+
+cleanup:
+    if (db) {
+        sqlite3_exec(db, "SELECT cloudsync_terminate();", NULL, NULL, NULL);
+        sqlite3_close(db);
+    }
+    if (cleanup_databases) {
+        file_delete_internal(dbpath);
+        // also clean up WAL and SHM files
+        char walpath[280];
+        snprintf(walpath, sizeof(walpath), "%s-wal", dbpath);
+        file_delete_internal(walpath);
+        snprintf(walpath, sizeof(walpath), "%s-shm", dbpath);
+        file_delete_internal(walpath);
+    }
+    return result;
+}
+
+// Authorizer state for do_test_context_cb_error_cleanup.
+// Denies INSERT on a specific table after allowing a set number of INSERTs.
+static const char *g_deny_insert_table = NULL;
+static int g_deny_insert_remaining = 0;
+
+static int deny_insert_authorizer(void *pUserData, int action, const char *zArg1,
+                                   const char *zArg2, const char *zDbName, const char *zTrigger) {
+    (void)pUserData; (void)zArg2; (void)zDbName; (void)zTrigger;
+    if (action == SQLITE_INSERT && g_deny_insert_table &&
+        zArg1 && strcmp(zArg1, g_deny_insert_table) == 0) {
+        if (g_deny_insert_remaining > 0) {
+            g_deny_insert_remaining--;
+            return SQLITE_OK;
+        }
+        return SQLITE_DENY;
+    }
+    return SQLITE_OK;
+}
+
+// Test that the error cleanup path in table_add_to_context_cb doesn't crash
+// when databasevm_prepare fails for the merge statement.
+// Before the zeroalloc fix, col_value_stmt[index] contained uninitialized
+// garbage and databasevm_finalize was called on it → SIGSEGV.
+bool do_test_context_cb_error_cleanup(void) {
+    sqlite3 *db = NULL;
+    cloudsync_context *ctx = NULL;
+    bool result = false;
+
+    int rc = sqlite3_open(":memory:", &db);
+    if (rc != SQLITE_OK) return false;
+    sqlite3_cloudsync_init(db, NULL, NULL);
+
+    // Create table with PK and non-PK columns
+    rc = sqlite3_exec(db,
+        "CREATE TABLE ctx_err (id TEXT PRIMARY KEY NOT NULL, val TEXT, num INTEGER);",
+        NULL, NULL, NULL);
+    if (rc != SQLITE_OK) goto cleanup;
+
+    // Init cloudsync on the table (creates meta table, triggers, settings)
+    rc = sqlite3_exec(db, "SELECT cloudsync_init('ctx_err');", NULL, NULL, NULL);
+    if (rc != SQLITE_OK) goto cleanup;
+
+    // Create a fresh context — ctx_err is not in its table list,
+    // so table_add_to_context won't short-circuit at table_lookup.
+    ctx = cloudsync_context_create(db);
+    if (!ctx) goto cleanup;
+
+    // Install authorizer that denies INSERT on the base table.
+    // Allow 1 INSERT (the sentinel prepared in table_add_stmts),
+    // then deny the next (the merge UPSERT prepared in the callback).
+    // This causes databasevm_prepare to fail inside table_add_to_context_cb,
+    // triggering the error cleanup path.
+    g_deny_insert_table = "ctx_err";
+    g_deny_insert_remaining = 1;
+    sqlite3_set_authorizer(db, deny_insert_authorizer, NULL);
+
+    // Must return false (callback error) without crashing
+    bool added = table_add_to_context(ctx, table_algo_crdt_cls, "ctx_err");
+
+    // Remove authorizer before any further operations
+    sqlite3_set_authorizer(db, NULL, NULL);
+    g_deny_insert_table = NULL;
+
+    if (added) goto cleanup;  // should have failed
+
+    result = true;
+
+cleanup:
+    sqlite3_set_authorizer(db, NULL, NULL);
+    g_deny_insert_table = NULL;
+    if (ctx) cloudsync_context_free(ctx);
+    if (db) {
+        sqlite3_exec(db, "SELECT cloudsync_terminate();", NULL, NULL, NULL);
+        sqlite3_close(db);
+    }
+    return result;
+}
+
 // Test cloudsync_terminate function
 bool do_test_terminate(void) {
     sqlite3 *db = NULL;
@@ -11082,6 +11231,8 @@ int main (int argc, const char * argv[]) {
     result += test_report("Delete/Resurrect Order:", do_test_delete_resurrect_ordering(3, print_result, cleanup_databases));
     result += test_report("Large Composite PK Test:", do_test_large_composite_pk(2, print_result, cleanup_databases));
     result += test_report("Schema Hash Mismatch:", do_test_schema_hash_mismatch(2, print_result, cleanup_databases));
+    result += test_report("Stale Table Settings:", do_test_stale_table_settings(cleanup_databases));
+    result += test_report("CB Error Cleanup:", do_test_context_cb_error_cleanup());
 
 finalize:
     if (rc != SQLITE_OK) printf("%s (%d)\n", (db) ? sqlite3_errmsg(db) : "N/A", rc);