[OpenSIPS-Devel] [ opensips-Patches-2044475 ] Matching operators fail to match on some platforms

SourceForge.net noreply at sourceforge.net
Sun Aug 10 22:46:35 CEST 2008


Patches item #2044475, was opened at 2008-08-10 03:47
Message generated for change (Comment added) made by bogdan_iancu
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=1086412&aid=2044475&group_id=232389

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: core
Group: 1.4.x
Status: Open
Resolution: Fixed
Priority: 5
Private: No
Submitted By: Amy Linari (amylinari)
Assigned to: Bogdan-Andrei Iancu (bogdan_iancu)
Summary: Matching operators fail to match on some platforms

Initial Comment:
Due to a bug in route.c, the regular expression matching operators do not
work in all environments. For example, when building 1.4.0 or 1.3.2 on
Solaris 10 (sun4v or x86) they give the same result independent of input.
It would seem that if it were not for the accident of a particular struct
alignment, these operators would be broken on all platforms.

The underlying problem is a bad test in comp_s2s, which expects s2 to be a
str*, and tests st->s==NULL. When handling the matching operators MATCH_OP
and NOTMATCH_OP, it is a regex_t* instead - a fact reflected in the cast
under the appropriate switch case.

The attached patch moves these special case tests into a new function
comp_s2r which handles MATCH_OP and NOTMATCH_OP comparisons with the
correct types and tests. The corresponding patch for kamailio 1.3.2 has been filed and tested. This version of the patch has NOT been tested, but should work properly.

----------------------------------------------------------------------

>Comment By: Bogdan-Andrei Iancu (bogdan_iancu)
Date: 2008-08-10 23:46

Message:
Logged In: YES 
user_id=1275325
Originator: NO

Hi Amy,

The fix I did do not discard the check on the s2 parameter - that will be
quite unsafe :O - I moved the check inside each "case" of the "switch" and
adept it to each specific op. 
As said I understand that your fix split the function in two (for regexp
and no regexp ops), but I preferred to keep it consistent and have only one
function.

I agree you the problem (as pointed out by you) with the improper param
check - the fix difference only :)

If you find something wrong in the fix I did, please let me know - I will
keep the report open.

Thanks and regards,
bogdan

----------------------------------------------------------------------

Comment By: Amy Linari (amylinari)
Date: 2008-08-10 17:27

Message:
Logged In: YES 
user_id=2112687
Originator: YES

Hi Bogdan,

Sorry to hear that patch didn't quite work. In any case, I expect your
changes will do the trick, though I can't speak as to the effect of the
discarded check on the s2 input. I separated the functions because the
alternative I could see was a ~10 line block to copy the third param into
the appropriate location (either s2 or re) and perform appropriate checks
on both. However, that tactic leaves some invalid pointers hanging about
and continues the practice of casting a pointer to force it through. It
seemed to me, after a bit of time, that comp_s2s was simply the wrong place
to compare a string to a regex, so I broke it out into a new function which
preserves all the input checks.

Being less that thoroughly familiar with the internals, I have to wonder
why those checks exist at all, and why they return 0, rather than
propagating an error, but I lacked the knowledge to judge whether such a
change would be appropriate.

-Amy

----------------------------------------------------------------------

Comment By: Bogdan-Andrei Iancu (bogdan_iancu)
Date: 2008-08-10 12:58

Message:
Logged In: YES 
user_id=1275325
Originator: NO

Hi Amy,

Thank you for pointing out the problem - unfortunately the uploaded patch
was incomplete, but i figured out what was the fixed you did :). I tried a
different version to keep a single function and to place specific param
tests inside the "switch".

Regards,
Bogdan

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=1086412&aid=2044475&group_id=232389



More information about the Devel mailing list