added param sip_uri_match 0 - case sensitive (default) 1 - case insensitive
can be extended to a more compliant version, (sensitive user, insensitive domain). the parameter sets the function to call to match uris.
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/94
-- Commit Summary --
* presence - match sip uris using alternative function
-- File Changes --
M modules/presence/hash.c (4) M modules/presence/notify.c (10) M modules/presence/presence.c (45) M modules/presence/presence.h (3) M modules/presence/presentity.c (2)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/94.patch https://github.com/kamailio/kamailio/pull/94.diff
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94
Are we still missing docs here?
Note that the param in registrar has another name. Since this is not about the full SIP uri- i hope we always are case insensitive for domains - maybe a better name would be sip_username_match
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-75101282
yes, i know the docs are missing.
when is something other then a fix, i will gladly add the docs if the request is accepted.
"i hope we always are case insensitive for domains"
this implementation acts on the full sip uri. it is open to add a 3rd option that will take the full uri and matches sensitive on the username and insensitive on the domain.
before this it acts on the full sip uri as case sensitive, i give it the alternative and left it open enough (i think) to implement full RFC compliance.
________________________________ From: sr-dev [sr-dev-bounces@lists.sip-router.org] on behalf of Olle E. Johansson [notifications@github.com] Sent: Thursday, February 19, 2015 9:53 AM To: Kamailio Devel List Subject: Re: [sr-dev] [kamailio] presence - match sip uris using alternative function (#94)
Are we still missing docs here?
Note that the param in registrar has another name. Since this is not about the full SIP uri- i hope we always are case insensitive for domains - maybe a better name would be sip_username_match
— Reply to this email directly or view it on GitHubhttps://github.com/kamailio/kamailio/pull/94#issuecomment-75101282.
If the module are comparing domains with case sensitivity today, that's a bug that needs to be fixed.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-75110709
There are helper C functions for comparing the URI, used by:
* http://kamailio.org/docs/modules/devel/modules/siputils.html#siputils.f.cmp_...
Not sure how this strict uri matching can be made to work with db only mode... but I guess that is a compromise.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-75123625
lazedo writes:
added param sip_uri_match 0 - case sensitive (default) 1 - case insensitive
should it rather be called sip_uri_user_match?
i still don't feel comfortable in starting to bloat kamailio with non-standard behavior. why don't you canonize uri user in config script?
-- juha
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-75129328
Olle E. Johansson writes:
If the module are comparing domains with case sensitivity today, that's a bug that needs to be fixed.
exactly. i also pointed this out earlier. if implemented, the new function must operate on uri user only, but i would not prefer to add this functionality at all as i mentioned in previous message.
-- juha
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-75130050
Olle,
it acts on the sip uri not the user part of the sip uri.
On Thu, Feb 19, 2015 at 8:27 PM, juha-h notifications@github.com wrote:
lazedo writes:
added param sip_uri_match 0 - case sensitive (default) 1 - case insensitive
should it rather be called sip_uri_user_match?
i still don't feel comfortable in starting to bloat kamailio with non-standard behavior. why don't you canonize uri user in config script?
-- juha
— Reply to this email directly or view it on GitHub https://github.com/kamailio/kamailio/pull/94#issuecomment-75129328.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-75130074
lazedo writes:
it acts on the sip uri not the user part of the sip uri.
and that exactly is the problem as we have tried to tell.
-- juha
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-75130859
@juha-h - because the comparison is done on different URIs (sender, target, contact), I am not sure that would be easy to canonise in config.
Looking at the patch I think it is in the right direction to get this sorted out according to RFC but also for other cases if needed.
Now is case sensitive, perhaps the default should be made to use the internal uri comparison functions.
The patch makes it easy to decide on what kind of comparison to do. There will be no side effect on performances if we go for uri comparison function or allow other options via mod param.
@lazedo - can you make the patch for the docs of the module to reflect the new parameter? I am considering to merge at some point, followed by adding the option for RFC uri comparison, eventually making that default option.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-75640451
@miconda sure. one question, shuld we also <abstract the core_hash used in the module to work the same way as the match function ? abstracting to core_hash / core_case_hash ?
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-75745275
What do you mean by abstracting the core_hash / core_case_hash ? To have a function inside presence module as a wrapper?
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-77713480
yes, working the same as the uri comparison, using function pointer.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-77822727
I think here we can go with case insensitive hashing, the function is not slower and covers properly all cases. Adding a new parameter will make it more complex, exposing something that is pure technical and related only to the internals of the module.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-77840747
ok, i wasn't thinking in adding a new param and use the previous one ( sip_uri_match ) to either call core_hash or core_case_hash. are you proposing to change the core_hash with core_case_hash independently from sip_uri_match param?
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-77841723
Yes, use core_case_hash() -- I think it is the best option: not adding any overhead and avoids extra config complexity.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-77842848
closing here because of rebase that didn't go well.
https://github.com/kamailio/kamailio/pull/116
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#issuecomment-86783698
Closed #94.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/94#event-266503710
-
Daniel-Constantin Mierla -
juha-h
-
lazedo
-
Luis Azedo -
Olle E. Johansson