[asterisk-dev] [Code Review] SIP URI comparison test (plus some bug fixes)

[Mark Michelson]

> On 2010-07-20 12:12:37, Matthew Nicholson wrote:
> > /trunk/channels/sip/reqresp_parser.c, lines 2165-2168
> > <https://reviewboard.asterisk.org/r/792/diff/4/?file=11690#file11690line2165>
> >
> >     Perhaps you should make the loop below check everything twice, once with uri1 and uri2 in that order and once with uri2 and uri1 (the reverse).

Great idea. This will help to remove some of the seemingly redundant test cases.


> On 2010-07-20 12:12:37, Matthew Nicholson wrote:
> > /trunk/channels/sip/reqresp_parser.c, line 2192
> > <https://reviewboard.asterisk.org/r/792/diff/4/?file=11690#file11690line2192>
> >
> >     You should test "sip" and "sip" for crashiness too.

Will do.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/792/#review2435
-----------------------------------------------------------


On 2010-07-20 11:31:14, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/792/
> -----------------------------------------------------------
> 
> (Updated 2010-07-20 11:31:14)
> 
> 
> Review request for Asterisk Developers and Olle E Johansson.
> 
> 
> Summary
> -------
> 
> Issue 17662 was opened by oej since he discovered that using a string comparison for domains was not a viable solution since IPv6 addresses may be represented in many different ways, all of which are considered to be the same address. The patch attached on that issue was my initial attempt at solving the issue.
> 
> While I was dabbling in the area, I felt it would be a good idea to add a SIP URI comparison internal test to be sure that all the logic was correct. What I found was that in many regards, the comparison of URI parameters was not working as it should be. That being the case, I largely rewrote the logic of sip_uri_params_cmp() to work correctly.
> 
> All SIP URI comparison code has been moved to channels/sip/reqresp_parser.c, including the new test. If you have good ideas for test cases that I haven't thought of yet, please feel free to add comments to the review request.
> 
> Also of note, the bugs I found in sip_uri_params_cmp() affect 1.4 and 1.6.2 as well. My plan is to backport the bugfixes to those branches as well once I get a "Ship it!" on this review.
> 
> 
> This addresses bug 17662.
>     https://issues.asterisk.org/view.php?id=17662
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 277873 
>   /trunk/channels/sip/include/reqresp_parser.h 277813 
>   /trunk/channels/sip/reqresp_parser.c 277813 
> 
> Diff: https://reviewboard.asterisk.org/r/792/diff
> 
> 
> Testing
> -------
> 
> The new set of tests passes.
> 
> 
> Thanks,
> 
> Mark
> 
>