[sr-dev] pua module db query bug

[Daniel-Constantin Mierla]

On 12/10/14 09:37, Juha Heinanen wrote:
> while doing pua module tests in db_mode=2, i used pua_mi to send two
> publish requests.  in both of them id and etag params had value '.'.
>
> first generated publish request without sip-if-match header (as it
> should) and after 200 ok was received, record was properly inserted to
> pua table with etag value a.1413029856.5521.112.0 and empty pres_id.
>
> the second for a different pres_uri however resulted in error in
> presence server:
>
> Oct 12 10:06:42 siika /usr/bin/pres-serv[5522]: ERROR: presence [presentity.c:807]: update_presentity(): No E_Tag match a.1413029856.5521.112.0
>
> after digging into it, i found with wireshark that the second publish
> indeed included sip-if-match header with that etag value even when it
> was initial publish (etag and id params '.') for another pres_uri.
>
> then i took a look at pua source and found that if etag param is null it
> is not included in db query at all leaving only pres_id param to match
> against:
>
> ua_pres_t *get_record_puadb(str pres_id, str *etag, ua_pres_t *result, db1_res_t **dbres)
> {
> 	int n_query_cols = 0, nr_rows;
>      ...
> 	q_cols[n_query_cols] = &str_pres_id_col;	
> 	q_vals[n_query_cols].type = DB1_STR;
> 	q_vals[n_query_cols].nul = 0;
> 	q_vals[n_query_cols].val.str_val = pres_id;
> 	n_query_cols++;
>
> 	if (etag != NULL)
> 	{
> 		q_cols[n_query_cols] = &str_etag_col;
> 		q_vals[n_query_cols].type = DB1_STR;
> 		q_vals[n_query_cols].nul = 0;
> 		q_vals[n_query_cols].val.str_val.s = etag->s;
> 		q_vals[n_query_cols].val.str_val.len = etag->len;
> 		n_query_cols++;
> 	}
>      ...
> 	if(query_fn(pua_db, q_cols, 0, q_vals,
> 				NULL,n_query_cols,0,0,&res) < 0)
> 	{
> 		LM_ERR("DB query error\n");
> 		return(NULL);
> 	}
>
> of course this kind of query matched the just inserted record for
> another pres_uri, which is wrong.
>
> how should this be fixed?  is it enough to always add etag value to the
> query or should also pres_uri be added just in case etags for different
> pres_uris would match?  or perhaps the best fix would be to skip the
> query altogether in send_publish if etag is null?
I think that if it is an initial publish, then send out directly (I 
assume a new etag is going to be generated).

Then, iirc, etag is kind of unique globally per instance if generated by 
kamailio, so if it exists, then a query on it would be safe. But if it 
is generated by somone else, perhaps we cannot guarantee that, therefore 
I think the query has to be done on pres_uri as well to be in safe side 
always.

In other words, my impression is that both updates have to be done.

Cheers,
Daniel
>
> has anyone else ever used pua module in db_mode=2?
>
> -- juha
>
> _______________________________________________
> sr-dev mailing list
> sr-dev at lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev

-- 
Daniel-Constantin Mierla
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda