uac_registrant: compare Contact URI case-insensitively #3263
Conversation
We had an issue where the response Contact URI had ";transport=udp" changed to ";transport=UDP". This commit solves the problem by comparing the Contact URI case-insensitively.
|
Maybe we can continue and improve this one #2558 ? |
| @@ -482,7 +482,7 @@ int run_reg_tm_cback(void *e_data, void *data, void *r_data) | |||
| while (contact) { | |||
| /* Check for binding */ | |||
| if (contact->uri.len==rec->contact_uri.len && | |||
| strncmp(contact->uri.s,rec->contact_uri.s,contact->uri.len)==0){ | |||
| strncasecmp(contact->uri.s,rec->contact_uri.s,contact->uri.len)==0){ | |||
There was a problem hiding this comment.
This is not correct because if the username was altered, the URIs are not the same:
sip:[email protected];Transport=udp (different usernames)
sip:[email protected];Transport=UDP
There was a problem hiding this comment.
Thanks, what would you think of adding a strncmp_lc_transport that takes a pkg_malloc copy of both strings, turns the ;transport=... into lowercase, then strcmps the copies with lowercased transport, and then frees the copies and returns the result of the strcmp?
There was a problem hiding this comment.
Take a look at compare_uris() from parse_uri.c.
It does the proper job if URIs are passed as parsed structures, but comparing raw URIs is bad :(
I pushed some fixes on the compare_uris branch:
https://github.com/OpenSIPS/opensips/tree/compare_uris
Please give it a try and let me know the results.
There was a problem hiding this comment.
Thank you for the work on this, yes this looks good to me!
There was a problem hiding this comment.
Did you test the new branch in production and it's working ok for you?
There was a problem hiding this comment.
Not in production, no, only in a test setup. I registered a user with incorrect case in the transport param and it worked & did not cause any obvious issues.
|
Any updates here? No progress has been made in the last 30 days, marking as stale. |
|
No updates here. |
|
Any updates here? No progress has been made in the last 30 days, marking as stale. |
|
No updates here. |
|
… On Sun, Feb 11, 2024 at 08:33 James Stanley ***@***.***> wrote:
No updates here.
—
Reply to this email directly, view it on GitHub
<#3263 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABA2XP6ADZ2LGTDFKX7Y4STYTDCDVAVCNFSM6AAAAABADLV2CKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZXG42TMMZQGM>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
Oh! Great success, thank you! |
|
Hi @ovidiusas @bogdan-iancu - how come this change didn't make it into 3.4.4? |
|
Reopening for visibility. |
|
Any updates here? No progress has been made in the last 30 days, marking as stale. |
|
It was merged but not included in the release? |
|
Any updates here? No progress has been made in the last 30 days, marking as stale. |
|
As per #3263 (comment) - a fix was merged but didn't make it into the release, what is happening there? |
|
Any updates here? No progress has been made in the last 30 days, marking as stale. |
|
Per https://www.opensips.org/pub/opensips/3.5.0-beta/ChangeLog it looks like this is going to make it into a release, closing. |
Summary
We had an issue where the response Contact URI had ";transport=udp" changed to ";transport=UDP".
Details
The unequal casing of "UDP" meant that
uac_registrantdid not recognise the Contact header, and therefore ignored the;expires=...value on the header, leading to OpenSIPS failing to re-register the user at the correct interval.Solution
This commit solves the problem by comparing the Contact URI case-insensitively.
Compatibility
I doubt this breaks any scenarios.
Closing issues