Skip to content

uac_registrant: compare Contact URI case-insensitively #3263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jes wants to merge 1 commit into from
Closed

uac_registrant: compare Contact URI case-insensitively #3263

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/uac_registrant/registrant.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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){
Copy link
Member

@ovidiusas ovidiusas Dec 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not correct because if the username was altered, the URIs are not the same:
sip:alice@AtLanTa.CoM;Transport=udp (different usernames)
sip:ALICE@AtLanTa.CoM;Transport=UDP

Copy link
Contributor Author

@jes jes Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

@ovidiusas ovidiusas Dec 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@jes jes Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the work on this, yes this looks good to me!

Copy link
Member

@ovidiusas ovidiusas Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test the new branch in production and it's working ok for you?

Copy link
Contributor Author

@jes jes Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (contact->expires && contact->expires->body.len) {
if (str2int(&contact->expires->body, &exp)<0) {
LM_ERR("Unable to extract expires from [%.*s]"
Expand Down