[sr-dev] tsilo - t_append() uri
Daniel-Constantin Mierla
miconda at gmail.com
Tue Sep 22 15:54:23 CEST 2015
Hello,
we can parse the parameter and if fails, then return false.
Not sure if use_domain would be needed anymore, I think it won't harm to
keep it. The main thing to solve here is that the uri parameter for
ts_append() is forwarded to lookup_to_dset().
Since I got to it, I attach a patch that adds uri parameter to
ts_append_to() and forwards the uri to lookup_to_dset(). ts_append_to()
can still be used without uri parameter, which will have same behaviour
like now
If you think it is ok, I can push it to git. The part with use_domain
can be decided later.
Cheers,
Daniel
On 22/09/15 15:48, Federico Cabiddu wrote:
> Hi,
> ok, now I understand :)
> What do you think would be the best way to enforce the usage of
> complete sip uris in ts_append()?
> Do you think that at this point "use_domain" would be useless in tsilo?
>
> Regards,
>
> Federico
>
> On Tue, Sep 22, 2015 at 3:37 PM, Daniel-Constantin Mierla
> <miconda at gmail.com <mailto:miconda at gmail.com>> wrote:
>
> And to add the relevant code:
>
> modules/tsilo/ts_append.c line 85:
>
> orig_msg = t->uas.request;
>
> ret = _regapi.lookup_to_dset(orig_msg, table, NULL);
> if(ret != 1) {
> LM_DBG("transaction %u:%u: error updating dset (%d)\n",
> tindex, tlabel, ret);
> return -1;
> }
>
> I want to pass the uri parameter given to t_append() to:
>
> ret = _regapi.lookup_to_dset(orig_msg, table, NULL);
>
> instead of NULL.
>
> Cheers,
> Daniel
>
>
> On 22/09/15 15:34, Daniel-Constantin Mierla wrote:
>> Hello,
>>
>> it was no longer about what tsilo stores internally, but what
>> lookup_to_dset() is given to search for new contact.
>>
>> To explain with an example:
>>
>> Call to sip:user at domain.
>>
>> After lookup("location"), the r-uri of INVITE is changed to
>> contact address found in location table (let's say it was only
>> one contact there, which is sip:randomstring at deviceip:deviceport).
>>
>> t_relay() is execute and transaction is created, with uas field
>> holding the INVITE with r-uri sip:randomstring at deviceip:deviceport.
>>
>> After a while, a device for same user registers, if I call
>> ts_append(..., "sip:user at domain"), it finds the INVITE
>> transaction to be associated with sip:user at domain, but
>> lookup_to_dset() is executed internally by tsilo with the
>> argument being the request from Transaction->uas (and no explicit
>> URI parameter). So the lookup is done for
>> sip:randomstring at deviceip:deviceport), because that is the r-uri
>> in T-uas reques. Obviously that will fail to find new contacts.
>>
>> Hopefully I could expose the issue better now.
>>
>> Cheers,
>> Daniel
>>
>> On 22/09/15 15:14, Federico Cabiddu wrote:
>>> Hi,
>>> I'm not sure I fully understand the problem. See my comments inline
>>>
>>> On Tue, Sep 22, 2015 at 2:24 PM, Daniel-Constantin Mierla
>>> <miconda at gmail.com <mailto:miconda at gmail.com>> wrote:
>>>
>>> Hello,
>>>
>>> tsilo is not using the uri parameter for looking up location
>>> for new
>>> destinations. It passes the uas request from transaction
>>> module to
>>> registrar lookup_to_dset() function, so the r-uri from that
>>> request is used.
>>>
>>> However, there is at least one scenario that makes tsilo not
>>> working
>>> properly:
>>>
>>> - invite comes in
>>> - lookup location finds a contact
>>> - request is forwarded with a new r-uri (the contact of the
>>> location
>>> record) -- t_relay() is used and trasaction created, r-uri
>>> stored for
>>> uas is contact header
>>> - a new registration comes and ts_append() is executed,
>>> failing to find
>>> contacts for r-uri in uas because the r-uri there can be
>>> anything
>>> pointing to the first device found in location
>>>
>>>
>>> This happens because since
>>> commit 1e5bad019c450a003e812ee051d84134aad6c5f0, we are using
>>> the current uri to store the transaction.
>>> That's why I was wondering (probably I have not been clear in my
>>> other mail :)) if, since now ts_store accepts uris as parameter,
>>> we still need this.
>>> In this case, the scenario you describe would work flawlessly
>>> and, if one need to store the transaction under a new ruri after
>>> some kind of message manipulation (like using alias etc) he can
>>> always call ts_store with the specific uri he wants to use.
>>> Am I missing something?
>>
>>>
>>> Some solutions can be found by creating transaction earlier,
>>> but not
>>>
>>> easy to tackle always.
>>>
>>> I think it would be better to just pass the uri parameter from
>>> ts_append() to lookup_to_dset(). But I noticed that the uri
>>> can be just
>>> username if use_domain is 0, which makes things a little
>>> more complex,
>>> because registrar/usrloc expect full sip uri.
>>>
>>> The solution would be to 'force' always that the uri
>>> parameters for
>>> tsilo functions are full SIP uri, and let the code extract
>>> only username
>>> when use_domain is 0. If someone has only the username in
>>> config, it can
>>> simply put the uri parameter as "sip:username at localhost" --
>>> domain will
>>> be ignored anyhow.
>>>
>>>
>>> Agree that this could avoid future headaches and provide more
>>> flexibility for other use cases. In this case, should we totally
>>> remove the dependency on usrloc "use_domain" parameter?
>>>
>>>
>>> Alternative, if the parameter is just username, built a
>>> temporary uri
>>> for it before passing to lookup_to_dset().
>>>
>>> I find first solution a bit better because the parameter for
>>> tsilo
>>> functions is called uri and building an internal one can end
>>> up in some
>>> limitations if it is going to be something that requires
>>> tel: or sips:
>>> or parameters such as instance.
>>>
>>>
>>> Agree.
>>>
>>>
>>> Any comments/suggestions?
>>>
>>> Cheers,
>>> Daniel
>>>
>>>
>>> Cheers,
>>>
>>> Federico
>>>
>>
>> --
>> Daniel-Constantin Mierla
>> http://twitter.com/#!/miconda <http://twitter.com/#%21/miconda> - http://www.linkedin.com/in/miconda
>> Book: SIP Routing With Kamailio - http://www.asipto.com
>> Kamailio Advanced Training, Sep 28-30, 2015, in Berlin - http://asipto.com/u/kat
>
> --
> Daniel-Constantin Mierla
> http://twitter.com/#!/miconda <http://twitter.com/#%21/miconda> - http://www.linkedin.com/in/miconda
> Book: SIP Routing With Kamailio - http://www.asipto.com
> Kamailio Advanced Training, Sep 28-30, 2015, in Berlin - http://asipto.com/u/kat
>
>
--
Daniel-Constantin Mierla
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Book: SIP Routing With Kamailio - http://www.asipto.com
Kamailio Advanced Training, Sep 28-30, 2015, in Berlin - http://asipto.com/u/kat
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20150922/f377d712/attachment-0001.html>
-------------- next part --------------
diff --git a/modules/tsilo/ts_append.c b/modules/tsilo/ts_append.c
index 1295f03..ba4b4c4 100644
--- a/modules/tsilo/ts_append.c
+++ b/modules/tsilo/ts_append.c
@@ -59,7 +59,7 @@ int ts_append(struct sip_msg* msg, str *ruri, char *table) {
while(ptr) {
LM_DBG("transaction %u:%u found for %.*s, going to append branches\n",ptr->tindex, ptr->tlabel, ruri->len, ruri->s);
- appended = ts_append_to(msg, ptr->tindex, ptr->tlabel, table);
+ appended = ts_append_to(msg, ptr->tindex, ptr->tlabel, table, ruri);
if (appended > 0)
update_stat(added_branches, appended);
ptr = ptr->next;
@@ -70,7 +70,7 @@ int ts_append(struct sip_msg* msg, str *ruri, char *table) {
return 1;
}
-int ts_append_to(struct sip_msg* msg, int tindex, int tlabel, char *table) {
+int ts_append_to(struct sip_msg* msg, int tindex, int tlabel, char *table, str *uri) {
struct cell *t;
struct sip_msg *orig_msg;
int ret;
@@ -84,7 +84,11 @@ int ts_append_to(struct sip_msg* msg, int tindex, int tlabel, char *table) {
orig_msg = t->uas.request;
- ret = _regapi.lookup_to_dset(orig_msg, table, NULL);
+ if(uri==NULL || uri->s==NULL || uri->len<=0) {
+ ret = _regapi.lookup_to_dset(orig_msg, table, NULL);
+ } else {
+ ret = _regapi.lookup_to_dset(orig_msg, table, uri);
+ }
if(ret != 1) {
LM_DBG("transaction %u:%u: error updating dset (%d)\n", tindex, tlabel, ret);
return -1;
diff --git a/modules/tsilo/ts_append.h b/modules/tsilo/ts_append.h
index 093449c..56f9120 100644
--- a/modules/tsilo/ts_append.h
+++ b/modules/tsilo/ts_append.h
@@ -23,6 +23,6 @@
#define _TS_APPEND_H
int ts_append(struct sip_msg* msg, str *ruri, char *table);
-int ts_append_to(struct sip_msg* msg, int tindex, int tlabel, char *table);
+int ts_append_to(struct sip_msg* msg, int tindex, int tlabel, char *table, str *uri);
#endif
diff --git a/modules/tsilo/tsilo.c b/modules/tsilo/tsilo.c
index f9bcfa9..fe475c6 100644
--- a/modules/tsilo/tsilo.c
+++ b/modules/tsilo/tsilo.c
@@ -61,6 +61,7 @@ static int mod_init(void);
static void destroy(void);
static int w_ts_append_to(struct sip_msg* msg, char *idx, char *lbl, char *d);
+static int w_ts_append_to2(struct sip_msg* msg, char *idx, char *lbl, char *d, char *ruri);
static int fixup_ts_append_to(void** param, int param_no);
static int w_ts_append(struct sip_msg* _msg, char *_table, char *_ruri);
static int fixup_ts_append(void** param, int param_no);
@@ -77,6 +78,8 @@ extern stat_var *added_branches;
static cmd_export_t cmds[]={
{"ts_append_to", (cmd_function)w_ts_append_to, 3,
fixup_ts_append_to, 0, REQUEST_ROUTE | FAILURE_ROUTE },
+ {"ts_append_to", (cmd_function)w_ts_append_to, 4,
+ fixup_ts_append_to, 0, REQUEST_ROUTE | FAILURE_ROUTE },
{"ts_append", (cmd_function)w_ts_append, 2,
fixup_ts_append, 0, REQUEST_ROUTE | FAILURE_ROUTE },
{"ts_store", (cmd_function)w_ts_store, 0,
@@ -226,6 +229,10 @@ static int fixup_ts_append_to(void** param, int param_no)
return -1;
}
}
+
+ if (param_no==4) {
+ return fixup_spve_null(param, 1);
+ }
return 0;
}
@@ -276,7 +283,34 @@ static int w_ts_append_to(struct sip_msg* msg, char *idx, char *lbl, char *table
return -1;
}
- return ts_append_to(msg, tindex, tlabel, table);
+ return ts_append_to(msg, tindex, tlabel, table, 0);
+}
+
+/**
+ *
+ */
+static int w_ts_append_to2(struct sip_msg* msg, char *idx, char *lbl, char *table, char *ruri)
+{
+ unsigned int tindex;
+ unsigned int tlabel;
+ str suri;
+
+ if(fixup_get_ivalue(msg, (gparam_p)idx, (int*)&tindex)<0) {
+ LM_ERR("cannot get transaction index\n");
+ return -1;
+ }
+
+ if(fixup_get_ivalue(msg, (gparam_p)lbl, (int*)&tlabel)<0) {
+ LM_ERR("cannot get transaction label\n");
+ return -1;
+ }
+
+ if(fixup_get_svalue(msg, (gparam_t*)ruri, &suri)!=0) {
+ LM_ERR("failed to conert r-uri parameter\n");
+ return -1;
+ }
+
+ return ts_append_to(msg, tindex, tlabel, table, &suri);
}
/**
More information about the sr-dev
mailing list