[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [osip-dev] implicit subscription

From: Aymeric Moizard
Subject: Re: [osip-dev] implicit subscription
Date: Wed, 28 Jun 2017 14:22:31 +0200


2017-06-26 23:28 GMT+02:00 FEICHTER Christoph <address@hidden>:



sorry – my fault.

everything fine !


your patch v4 partly works.

Nevertheless I have some improvements / hardening issues; attached as v5.


here are my explanations:


-          jcallback.c, function cb_rcv2xx
for performance reasons I put the handling of NOTIFY and REFER in an else-if cascade


-          jcallback.c, function cb_rcv2xx
eXcall_api.c, function
I re-worked the parsing a bit:

o   searching for the expires-parameter did not work, due to the different length of “active” and “pending”

o   you forgot to take into account the “;” between “active”/”pending” and the “expires” parameter

o   the length of “expires” is 7, not 6 chars
à I added const char’s for the strings to search for; so we can use strlen to get the right length

Please check again my version!!!

///CHECK 6 char for "active" or 7 char for "pending"
        if (0 == osip_strncasecmp (sub_state->hvalue, "active", 6)
          ||0 == osip_strncasecmp (sub_state->hvalue, "pending", 7)) {

//In the next strstr call, we are searching STARTING FROM HVALUE+6 for "expires" occurence.
  -> this means we start on the ";expires=180" if "active" was found.
  -> this means we start of the g;expires=180" if "pending" was found.

            const char *tmp = strstr(sub_state->hvalue+6, "expires");

-> in both case, the first occurence is found and tmp pointer is on first letter of "expires"

            const char *ss_expires = NULL;
            if (tmp!=NULL) {

//Here, we search for "=" starting 7 letter after "expires", so in both case, we are on "=" and ss_expires
//ends up being at tmp+7
              ss_expires = strchr(tmp+7, '=');

-> conclusion, my code handles correctly both "active" and "pending".
-> my code handle correctly any additionnal space before or after ";" and any other before or after "=".
-> this is why I do prefer my code! see below for more info on SEMI, EQUAL...

o   I did not like the double “if (ss_expires!=NULL)

Accepted! ;)

o   I use “;expires=” for searching for the expires-parameter

My code modification were made to accept any space (and LWS) inside the string! As I pointed above. 
Looking at rfc3265, here is the definition of Subscription-State

Subscription-State   = "Subscription-State" HCOLON substate-value
                          *( SEMI subexp-params )
   substate-value       = "active" / "pending" / "terminated"
                          / extension-substate
subexp-params        =   ("reason" EQUAL event-reason-value)
                          / ("expires" EQUAL delta-seconds)
                          / ("retry-after" EQUAL delta-seconds)
                          / generic-param

I have no much doubt that SEMI and EQUAL can contains space and other LWS...

o   check “sub_state->hvalue != NULL” was missing
otherwise crash in case of empty “Subscription-State” header

Good point! 

o   check “refer_sub->hvalue != NULL” was missing
otherwise crash in case of empty “Refer-Sub” header

Good point! 

-          Although the expires parameter should be present in the Subscription-State header of the NOTIFY
I added to use the default (
excontext->implicit_subscription_expires) in this case.

Right. My patch didn't fallback to default. I have modified my version to include that

jd->implicit_subscription_expire_time = now + excontext->implicit_subscription_expires;

I also moved the log to show when expires is not present. Also, my code also use default
value when expires is not showing a good value... (negative one or above 600.)

Additionnal change:
At the end of "eXosip_call_terminate_with_reason", your code do not include my change:
when a subscription exist, it will remove the dialog when sending a BYE.
Instead my code avoid to delete the dialog if implicit_subscription_expire_time has a value.

This is required to send NOTIFY after sending a BYE.

Other minor udp.c change were accepted!

Here is the newer v6 patch...


I should commit it by tomorrow. However, if anything is wrong let me know!







reply via email to

[Prev in Thread] Current Thread [Next in Thread]