From 7128c16e73c348d2b88a6980f7045d0628751b8d Mon Sep 17 00:00:00 2001 From: Andrea Donetti Date: Wed, 1 Apr 2026 15:38:58 -0600 Subject: [PATCH 1/5] fix: skip stale table settings for dropped tables during init When a table was dropped without calling cloudsync_cleanup(), its settings entry persisted as an orphan. On next init, trigger creation failed with "no such table" error. Add a database_table_exists() check before creating triggers to gracefully skip stale entries. --- src/dbutils.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/dbutils.c b/src/dbutils.c index 67bfeb8..696bc49 100644 --- a/src/dbutils.c +++ b/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)); From b652757c4dce834cf908f3eed428c64324dd4ee3 Mon Sep 17 00:00:00 2001 From: Andrea Donetti Date: Wed, 1 Apr 2026 15:41:30 -0600 Subject: [PATCH 2/5] fix: add col_name to cloudsync_table_settings primary key in SQLite The SQLite schema used PRIMARY KEY(tbl_name, key), while PostgreSQL correctly used PRIMARY KEY(tbl_name, col_name, key). The missing col_name caused REPLACE INTO to silently overwrite column-level settings sharing the same (tbl_name, key) pair. --- src/sqlite/sql_sqlite.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sqlite/sql_sqlite.c b/src/sqlite/sql_sqlite.c index 236a67b..7228579 100644 --- a/src/sqlite/sql_sqlite.c +++ b/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)"; From e6146208f65b0c3a5e5f08ce523a80d0d1fc332e Mon Sep 17 00:00:00 2001 From: Andrea Donetti Date: Wed, 1 Apr 2026 16:59:28 -0600 Subject: [PATCH 3/5] fix: zero-initialize column arrays to prevent crash on error cleanup col_merge_stmt and col_value_stmt were allocated with cloudsync_memory_alloc (unzeroed). If an error occurred before the prepared statements were assigned, the cleanup handler called databasevm_finalize on uninitialized garbage pointers, causing a SIGSEGV. Changed all four column arrays to cloudsync_memory_zeroalloc to match col_algo and col_delimiter. --- src/cloudsync.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/cloudsync.c b/src/cloudsync.c index e673b5f..ace2178 100644 --- a/src/cloudsync.c +++ b/src/cloudsync.c @@ -1079,16 +1079,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)); From dbe4ad3a6c728ae41f1206390972a9125798fc3e Mon Sep 17 00:00:00 2001 From: Andrea Donetti Date: Wed, 1 Apr 2026 17:00:11 -0600 Subject: [PATCH 4/5] fix: free column arrays in table_free regardless of ncols When table_add_to_context_cb failed before incrementing ncols, the column arrays (col_name, col_merge_stmt, etc.) were allocated but never freed because table_free guarded their cleanup with if (ncols > 0). Removed the guard so arrays are always freed. The for loops safely execute zero iterations when ncols is 0. Added a test that uses sqlite3_set_authorizer to force databasevm_prepare to fail inside the callback, exercising the error cleanup path that previously crashed on uninitialized pointers. --- src/cloudsync.c | 52 ++++++++--------- test/unit.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+), 27 deletions(-) diff --git a/src/cloudsync.c b/src/cloudsync.c index ace2178..7239428 100644 --- a/src/cloudsync.c +++ b/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; incols; ++i) { - cloudsync_memory_free(table->col_name[i]); - } - cloudsync_memory_free(table->col_name); - } - if (table->col_merge_stmt) { - for (int i=0; incols; ++i) { - databasevm_finalize(table->col_merge_stmt[i]); - } - cloudsync_memory_free(table->col_merge_stmt); + if (table->col_name) { + for (int i=0; incols; ++i) { + cloudsync_memory_free(table->col_name[i]); } - if (table->col_value_stmt) { - for (int i=0; incols; ++i) { - databasevm_finalize(table->col_value_stmt[i]); - } - cloudsync_memory_free(table->col_value_stmt); - } - 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; incols; ++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; incols; ++i) { + databasevm_finalize(table->col_value_stmt[i]); } - if (table->col_delimiter) { - for (int i=0; incols; ++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; incols; ++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); diff --git a/test/unit.c b/test/unit.c index b62fa60..77cc783 100644 --- a/test/unit.c +++ b/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); From 202439a6e6d548ebeed8cacf23f95861605f95b5 Mon Sep 17 00:00:00 2001 From: Andrea Donetti Date: Wed, 1 Apr 2026 17:34:03 -0600 Subject: [PATCH 5/5] chore: bump version --- src/cloudsync.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cloudsync.h b/src/cloudsync.h index a0d17ae..b9e09fa 100644 --- a/src/cloudsync.h +++ b/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