- fix improper length check You can merge this Pull Request by running:
git pull https://github.com/seudin/kamailio siputil_fix
Or you can view, comment on it, or merge it online at:
https://github.com/kamailio/kamailio/pull/28
-- Commit Summary --
* siputils: fix for e164_check()
-- File Changes --
M modules/siputils/checks.c (2)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/28.patch https://github.com/kamailio/kamailio/pull/28.diff
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/28
Merged #28.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/28#event-217674703
Note for the future -- be sure the comments on the commits are providing proper details.
While the description here was correct, respectively:
"fix improper length check"
The commit has something else:
"- the condition for non-digit matching was always false"
Which can be a side effect in some cases, but not the real bug that was fixed. The commit didn't change anything that tested if a char is between 0 and 9.
In the future, be sure the commit message is giving the proper details, otherwise it can mislead. Commit logs are very important, more than the comments on issue tracker.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/28#issuecomment-69650662
My bad. You're totally right.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/28#issuecomment-69705857
This regression appears to be in the 4.2.2 release, but not in 4.2.1 - are you likely to be releasing a 4.2.3 soon, or should we just go back to 4.2.1 for now?
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/28#issuecomment-70090218
It is not a regression, the fix was for an issue that is there for long time (including 4.2.1). The description is wrong and the fix doesn't relate to the previous patch fixing the 0-9 range check. There will be a 4.2.3, you can use git branch or nightly deb builds of branch 4.2 from apt repository.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/28#issuecomment-70100063
I'm not 100% sure that the issue was in 4.2.1, but I might be mistaken. I upgraded from 4.2.1 to 4.2.2 today and the first and only thing that broke was the `is_e164()` function always returning false. That was with inputs that had previously returned true in 4.2.1. But potentially I got something else wrong in my particular setup.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/28#issuecomment-70100819
The issue solved by a previous patch was that is_e164() was returning always true -- it was checking that (c<'0' && c>'9') to say it is not e164, which never happens.
The issue solved by the patch on this item was about going over the length of the parameter with 1, which was even before the first patch.
After the first patch, the effect of second issue going over the length, results in checking an extra character which very likely is not a digit.
In other words, the first fix revealed another bug by enabling a side effect of it in most of the cases.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/28#issuecomment-70110158
Btw, the check can be done with regex, like matching $rU is starting with + followed by [1-9] followed by many [0-9].
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/28#issuecomment-70110629
-
Ben Hood -
Daniel-Constantin Mierla
-
Seudin Kasumovic
-
Victor Seva