Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 32 additions & 34 deletions src/cloudsync.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion src/cloudsync.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions src/dbutils.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion src/sqlite/sql_sqlite.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)";
Expand Down
151 changes: 151 additions & 0 deletions test/unit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Loading