Re: [sr-dev] [Kamailio-Users] PV bug in modules_k/acc/acc_extra.c in K 3.0.0

I ran into a very interesting problem last night with 'acc' that took me a few hours to solve, so I am posting my workaround for collective benefit. It is probably not the cleanest and most appropriate solution because I am not extensively familiar with the 'acc' code; this is my first dive into it. Thus, I do not presume to submit an actual patch. The problem I had was that PVs read from the db_extra parameter that had an internal type of integer (TYPE_INT) because of their lexical scope were not being correctly evaluated, resulting in bizarre values being put in the database. I am using K 3.0.0 on 64-bit Xen guest running CentOS with db_postgres. I did not check to see if this problem occurs with db_mysql as I do not have MySQL anywhere nearby, but I don't believe it is happening at the database layer anyhow. I also know this problem was not present in 1.5.x, or several of my installations would be non-functional; this must have something to do with use of new 'srdb1' stuff in the database side of 'acc', I hypothesise. Anyway, the actual problem went like this. Let us say, for simplicity of example, that I am using 'acc' with Postgres backing and have something like this: modparam("acc", "db_extra", "account_id=$avp(s:account_id)") ... modparam("acc", "db_flag", 1) What was happening was that if I did this in a request route: $avp(s:account_id) = 42; ... setflag(1); if(!t_relay()) sl_reply_error(); exit; Very strange and occasionally bizarre values, mostly duplicates of other values I have in db_extra, would be written to acc.account_id (a column of type 'integer') - at any rate, certainly not 42. I enabled SQL statement logging on my PostgreSQL database to ensure that the data being inserted was literally not 42, rather than this being some sort of database-layer type conversion / representation problem. Sure enough, it was true, although I also did notice that 'acc' encloses all inserted values in single quotes to be treated as string literals. But this is immaterial, because RDBMs automatically cast such input: INSERT INTO acc (..., account_id) VALUES(..., '12'); Now, I could fix the problem by simply assigning the AVP a string literal instead: $avp(s:account_id) = '42'; Now the data was inserted normally. I also tested with user variables ($var(...)) and the same was true, so the problem is not AVP-specific. Also, all other evaluative contexts for PVs with these values work fine, such as printing them. The problem does not seem to be with 'db_postgres', neither with 'pv', nor with 'avpops' -- it is particular to 'acc'. After some hours of investigation I found that what was happening is that the type of the PV being referenced is not being correctly packed into the various arrays populated by extra2strar() in modules_k/acc/acc_extra.c, and thus was being misread when iterated in acc_db_request() in modules_k/acc/acc.c. Specifically, this check was inadequate: if (value.rs.s+value.rs.len==static_detector) { val_arr[n].s = int_buf + r*INT2STR_MAX_LEN; val_arr[n].len = value.rs.len; memcpy(val_arr[n].s, value.rs.s, value.rs.len); r++; LM_INFO("New int tenant: %s\n", val_arr[n].s); } else { val_arr[n] = value.rs; LM_INFO("New str tenant: %s\n", value.rs); } In my debugging, all PV values were showing up as "New str tenant" regardless of whether they were natively integers, and regardless of what the PV spec type was actually set to. The type was set correctly on the PV spec, so of course the integer value got stored correctly in int_var[] below: if (value.flags&PV_VAL_INT) { int_arr[n] = value.ri; type_arr[n] = TYPE_INT; } else { type_arr[n] = TYPE_STR; } ... but since 'acc' wants to treat all extra values as strings, this was not sufficient, because what was failing was this condition: if (value.rs.s+value.rs.len==static_detector) I guess value.rs.s in case of the PV value being PV_VAL_INT is not null terminated properly? This I truly do not know. In any event, I just added a check on the PV flags to see if it is an integer value and this fixed my problem: if (value.rs.s+value.rs.len==static_detector || value.flags & PV_VAL_INT) { ... however, I do not know that this was the most insightful solution. This is rather critical I think, because it means anyone trying to store PV values that are initialised with a natively numerical type in script (or extracted as a numerical value from a database, e.g. sql_query("SELECT account_id::integer FROM table ..."), I found this to be a problem as well) cannot store them in the database reliably using 'db_extra'.