[Kamailio-Users] PV bug in modules_k/acc/acc_extra.c in K 3.0.0
Daniel-Constantin Mierla
miconda at gmail.com
Tue Feb 2 12:59:06 CET 2010
On 2/2/10 7:43 AM, Alex Balashov wrote:
> 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'.
>
I will look into int. Is the db column string or integer?
Cheers,
Daniel
--
Daniel-Constantin Mierla
eLearning class for Kamailio 3.0.0
Starting Feb 8, 2010
* http://www.asipto.com/
More information about the Users
mailing list