From 86419e0e84b47600f187fd342d3572879dcad2cb Mon Sep 17 00:00:00 2001 From: Alessio Garzi Date: Wed, 4 Jun 2025 15:45:19 +0200 Subject: [PATCH] MT#62931 Append marking for removal to etag in presentity table The function mark_presentity_for_delete() is responsible for flagging a record in the presentity table for later removal. Previously, this was done by overwriting the etag field with a fixed, hardcoded string: static str str_offline_etag_val = str_init("*#-OFFLINE-#*"); However, the etag is intended to uniquely identify each PUBLISH message. Overwriting it with a constant value violates the following MySQL uniqueness constraint: UNIQUE KEY presentity_idx (username, domain, event, etag) For example, when handling multiple PUBLISH messages (e.g., for legA and legB of the same call), the fields: - username - domain - event are identical, and uniqueness is ensured solely by the etag. Replacing it with a fixed string may cause key collisions and trigger MySQL errors. To address this without altering the database schema or indexes, str_offline_etag_val is now appended to etag to retain its uniqueness. Change-Id: Icfff5da5dbaae1c47b4a0f33904a64f3b98ea957 (cherry picked from commit 77dff8f9b296ec913da52668fafe90cdd0d23bbf) (cherry picked from commit 4b679e350c66204a6acd4783d3ea87082a6add96) --- debian/patches/series | 1 + .../sipwise/presence_offline_cleanup.patch | 157 ++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 debian/patches/sipwise/presence_offline_cleanup.patch diff --git a/debian/patches/series b/debian/patches/series index a427561a2..ba034b6aa 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -59,6 +59,7 @@ sipwise/cfgutils-allow-lock_set_size-14.patch sipwise/pua-get_record_puadb-add-pres_uri-to-the-query.patch sipwise/pua_dialoginfo-use_uuid.patch sipwise/usrloc-don-t-synchronize-on-destroy-for-DB_ONLY.patch +sipwise/presence_offline_cleanup.patch ### active development sipwise/permissions-don-t-allow-reloads-in-the-middle-of-ong.patch ### Don't just put stuff in any order diff --git a/debian/patches/sipwise/presence_offline_cleanup.patch b/debian/patches/sipwise/presence_offline_cleanup.patch new file mode 100644 index 000000000..2aaabc793 --- /dev/null +++ b/debian/patches/sipwise/presence_offline_cleanup.patch @@ -0,0 +1,157 @@ +From: Sipwise Development Team +Date: Fri, 6 Jun 2025 08:55:40 +0200 +Subject: presence: fix presentity_offline_cleanup + +--- + src/modules/presence/presentity.c | 90 +++++++++++++++++++++++---------------- + 1 file changed, 54 insertions(+), 36 deletions(-) + +diff --git a/src/modules/presence/presentity.c b/src/modules/presence/presentity.c +index ac3f734..644c9b7 100644 +--- a/src/modules/presence/presentity.c ++++ b/src/modules/presence/presentity.c +@@ -2273,6 +2273,8 @@ int mark_presentity_for_delete(presentity_t *pres, str *ruid) + str *cur_body = NULL, *new_body = NULL; + db_query_f query_fn = pa_dbf.query_lock ? pa_dbf.query_lock : pa_dbf.query; + ++ LM_DBG("mark_presentity_for_delete called\n"); ++ + if(pres->event->agg_nbody == NULL) { + /* Nothing clever to do here... just delete */ + if(delete_presentity(pres, NULL) < 0) { +@@ -2359,6 +2361,8 @@ int mark_presentity_for_delete(presentity_t *pres, str *ruid) + goto done; + } + ++ LM_DBG("Found 1 presentity as expected\n"); ++ + row = RES_ROWS(result); + value = ROW_VALUES(row); + +@@ -2378,7 +2382,25 @@ int mark_presentity_for_delete(presentity_t *pres, str *ruid) + update_cols[n_update_cols] = &str_etag_col; + update_vals[n_update_cols].type = DB1_STR; + update_vals[n_update_cols].nul = 0; +- update_vals[n_update_cols].val.str_val = str_offline_etag_val; ++ str fb = {0, 0}; ++ if(pres->etag.len >= str_offline_etag_val.len ++ && memcmp(pres->etag.s + pres->etag.len - str_offline_etag_val.len, ++ str_offline_etag_val.s, str_offline_etag_val.len) ++ == 0) { ++ // Etag already has the offline suffix, just duplicate it ++ update_vals[n_update_cols].val.str_val = pres->etag; ++ LM_DBG("Etag already has the offline suffix, using it: %.*s\n", ++ pres->etag.len, pres->etag.s); ++ } else { ++ // Suffix not present, let's append it ++ if(str_append(&pres->etag, &str_offline_etag_val, &fb) != 0) { ++ LM_ERR("Can't append offline mark to etag\n"); ++ goto error; ++ } ++ update_vals[n_update_cols].val.str_val = fb; ++ LM_DBG("Etag without offline suffix, appending it: %.*s\n", fb.len, ++ fb.s); ++ } + n_update_cols++; + + update_cols[n_update_cols] = &str_expires_col; +@@ -2398,9 +2420,13 @@ int mark_presentity_for_delete(presentity_t *pres, str *ruid) + n_query_cols, n_update_cols) + < 0) { + LM_ERR("unsuccessful sql update operation"); ++ if(fb.len && fb.s) ++ pkg_free(fb.s); + goto error; + } + ++ if(fb.len && fb.s) ++ pkg_free(fb.s); + if(pa_dbf.affected_rows) + ret = pa_dbf.affected_rows(pa_db); + else +@@ -2510,12 +2536,13 @@ int delete_presentity(presentity_t *pres, str *ruid) + } + } + ++#define QUERY_LEN 1024 + int delete_offline_presentities(str *pres_uri, pres_ev_t *event) + { +- db_key_t query_cols[4]; +- db_val_t query_vals[4]; +- int n_query_cols = 0; + struct sip_uri uri; ++ str query_str = STR_NULL; ++ db1_res_t *res = NULL; ++ static char query[QUERY_LEN]; + + if(pa_db == NULL) { + LM_ERR("no database connection setup\n"); +@@ -2526,45 +2553,36 @@ int delete_offline_presentities(str *pres_uri, pres_ev_t *event) + LM_ERR("failed to parse presentity uri\n"); + goto error; + } ++ LM_DBG("presentity uri.user:%.*s\n", STR_FMT(&uri.user)); + +- query_cols[n_query_cols] = &str_username_col; +- query_vals[n_query_cols].type = DB1_STR; +- query_vals[n_query_cols].nul = 0; +- query_vals[n_query_cols].val.str_val = uri.user; +- n_query_cols++; +- +- query_cols[n_query_cols] = &str_domain_col; +- query_vals[n_query_cols].type = DB1_STR; +- query_vals[n_query_cols].nul = 0; +- query_vals[n_query_cols].val.str_val = uri.host; +- n_query_cols++; +- +- query_cols[n_query_cols] = &str_event_col; +- query_vals[n_query_cols].type = DB1_STR; +- query_vals[n_query_cols].nul = 0; +- query_vals[n_query_cols].val.str_val = event->name; +- n_query_cols++; +- +- query_cols[n_query_cols] = &str_etag_col; +- query_vals[n_query_cols].type = DB1_STR; +- query_vals[n_query_cols].nul = 0; +- query_vals[n_query_cols].val.str_val = str_offline_etag_val; +- n_query_cols++; +- +- if(pa_dbf.use_table(pa_db, &presentity_table) < 0) { +- LM_ERR("unsuccessful use table sql operation\n"); ++ if(!DB_CAPABILITY(pa_dbf, DB_CAP_RAW_QUERY)) { ++ LM_ERR("Database does not support raw queries"); + goto error; + } + +- if(pa_dbf.delete(pa_db, query_cols, 0, query_vals, n_query_cols) < 0) { ++ memset(query, 0, QUERY_LEN); ++ query_str.len = snprintf(query, QUERY_LEN, ++ "DELETE FROM %.*s WHERE " ++ "%.*s = '%.*s' AND " ++ "%.*s = '%.*s' AND " ++ "%.*s = '%.*s' AND " ++ "%.*s LIKE '%%%.*s'", ++ STR_FMT(&presentity_table), STR_FMT(&str_username_col), ++ STR_FMT(&uri.user), STR_FMT(&str_domain_col), STR_FMT(&uri.host), ++ STR_FMT(&str_event_col), STR_FMT(&event->name), ++ STR_FMT(&str_etag_col), STR_FMT(&str_offline_etag_val)); ++ ++ query_str.s = query; ++ LM_DBG("query:%.*s\n", STR_FMT(&query_str)); ++ ++ if(pa_dbf.raw_query(pa_db, &query_str, &res) < 0 || res == NULL) { + LM_ERR("unsuccessful sql delete operation"); +- goto error; ++ return -1; + } ++ LM_DBG("raw query executed successfully\n"); + +- if(pa_dbf.affected_rows) +- return pa_dbf.affected_rows(pa_db); +- else +- return 0; ++ pa_dbf.free_result(pa_db, res); ++ return 0; + + error: + return -1;