[sr-dev] [tracker] Task opened: Caching $hdr psuedo-variables causes a error when called from LUA

[sip-router]

THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY.

A new Flyspray task has been opened.  Details are below. 

User who did this - Hugh Waite (hugh.waite) 

Attached to Project - sip-router
Summary - Caching $hdr psuedo-variables causes a error when called from LUA
Task Type - Bug Report
Category - Modules kamailio
Status - Unconfirmed
Assigned To - 
Operating System - All
Severity - High
Priority - Normal
Reported Version - Development
Due in Version - Undecided
Due Date - Undecided
Details - We have been using the sr.pv.get() LUA function to extract custom SIP headers, but this has inconsistant results.

I have tracked this back to a bug in the pv module. However, I am not familiar enough with the pv_cache/pv_spec structure to know how to fix this.

In modules_k/pv/pv_core.c:pv_parse_hdr_name() the name of a header to look up is parsed and stored in the pv_spec_t structure. If this is an 'unknown' header, the string pointer is stored in the sructure (line 2487).

2478         sp->pvp.pvn.type = PV_NAME_INTSTR;
2479         if (hdr.type!=HDR_OTHER_T && hdr.type!=HDR_ERROR_T)
2480         {
2481                 LM_DBG("using hdr type (%d) instead of <%.*s>\n",
2482                         hdr.type, in->len, in->s);
2483                 sp->pvp.pvn.u.isname.type = 0;
2484                 sp->pvp.pvn.u.isname.name.n = hdr.type;
2485         } else {
2486                 sp->pvp.pvn.u.isname.type = AVP_NAME_STR;
2487                 sp->pvp.pvn.u.isname.name.s = *in;                 <-------
2488         }
2489         return 0;
2490 error:
2491         return -1;
2492 }

If this has been called from LUA (modules/app_lua/app_lua_sr.c:sr_lua_pv_get():895) this string is part of the lua environment which goes out of scope - causing a problem.

 883         pvn.s = (char*)lua_tostring(L, -1);
 884         if(pvn.s==NULL || env_L->msg==NULL)
 885                 return 0;
 886 
 887         pvn.len = strlen(pvn.s);
 888         LM_DBG("pv get: %s\n", pvn.s);
 889         pl = pv_locate_name(&pvn);
 890         if(pl != pvn.len)
 891         {
 892                 LM_ERR("invalid pv [%s] (%d/%d)\n", pvn.s, pl, pvn.len);
 893                 return 0;
 894         }
 895         pvs = pv_cache_get(&pvn);                                     <------------- Local variable passed in
 896         if(pvs==NULL)
 897         {
 898                 LM_ERR("cannot get pv spec for [%s]\n", pvn.s);
 899                 return 0;
 900         }

When the pv cache structure is created, memory is allocated to duplicate the PV name(pvapi.c:269). Is it safe to pass the newly allocated string into the parse_pv_spec function?

For example: (this is modified from the trunk version)
 257 pv_spec_t* pv_cache_add(str *name)
 258 {
 259         pv_cache_t *pvn;
 260         unsigned int pvid;
 261         char *p;
 262 
 263         if(_pv_cache_set==0)
 264         {
 265                 LM_DBG("PV cache not initialized, doing it now\n");
 266                 pv_init_cache();
 267         }
 268         pvid = get_hash1_raw(name->s, name->len);
 269         pvn = (pv_cache_t*)pkg_malloc(sizeof(pv_cache_t) + name->len + 1);
 270         if(pvn==0)
 271         {
 272                 LM_ERR("no more memory\n");
 273                 return NULL;
 274         }
 275         memset(pvn, 0, sizeof(pv_cache_t) + name->len + 1);          <------- Note: There is a separate initialisation bug on this line. It uses sizeof(pv_item_t) in trunk!
 276         pvn->pvname.len = name->len;
 277         pvn->pvname.s = (char*)pvn + sizeof(pv_cache_t);
 278         memcpy(pvn->pvname.s, name->s, name->len);
 279         p = pv_parse_spec(&pvn->pvname, &pvn->spec);                 <------- Is this safe?
 280 
 281         if(p==NULL)
 282         {
 283                 pkg_free(pvn);
 284                 return NULL;
 285         }
 286         pvn->pvid = pvid;
 287         pvn->next = _pv_cache[pvid%PV_CACHE_SIZE];
 288         _pv_cache[pvid%PV_CACHE_SIZE] = pvn;
 289 
 290         LM_DBG("pvar [%.*s] added in cache\n", name->len, name->s);
 291         return &pvn->spec;
 292 }

More information can be found at the following URL:
http://sip-router.org/tracker/index.php?do=details&task_id=252

You are receiving this message because you have requested it from the Flyspray bugtracking system.  If you did not expect this message or don't want to receive mails in future, you can change your notification settings at the URL shown above.