|
From: | FEICHTER Christoph |
Subject: | Re: [osip-dev] implicit subscription |
Date: | Wed, 28 Jun 2017 12:56:49 +0000 |
Hi, thanks for the explanations of your implementatioin. I was not aware of the optional spaces before/after the “;” and “=”. (I thought I could have it with higher performance using a single osip_strcasecmp
instead of calling
osip_strncasecmp, strstr and strchr.) thus, your implementation for parsing is fine – ACK ! ACK also for your additional changes. From my point of view we are ready for commit. Br, christoph From: Aymeric Moizard [mailto:address@hidden
inline! 2017-06-26 23:28 GMT+02:00 FEICHTER Christoph <address@hidden>:
ok!
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...
Accepted! ;)
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...
Good point!
Good point!
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! Regards Aymeric
|
[Prev in Thread] | Current Thread | [Next in Thread] |