[asterisk-dev] [Code Review]: chan_sip: removing some non-compliant code in v10 and minor fixes

wdoekes reviewboard at asterisk.org
Thu Oct 20 03:03:35 CDT 2011



> On Oct. 19, 2011, 2:12 p.m., Paul Belanger wrote:
> > Not sure about the SIP RFC however it would be nice to see some unit tests for these cases.  I think they could be added into the sip_uri parsing tests.

I'm not sure which tests you mean. "sip:host" and "sip:name at host" tests already exist in `sip_parse_uri_test`. Testing register_verify/check_user_full functionality is a bit beyond the scope of this patch (and should be in chan_sip.c, not in the sip_uri parsing tests).


- wdoekes


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


On Oct. 19, 2011, 12:30 p.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1533/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2011, 12:30 p.m.)
> 
> 
> Review request for Asterisk Developers and Paul Belanger.
> 
> 
> Summary
> -------
> 
> See the bug report: there were some XXX'es in the code about code that should be removed.
> 
> The patch does this:
> 
> (1) register_verify won't accept a To: without user-part anymore (illegal according to rfc3261, 10.2)
> 
> (2) check_user_full still doesn't require a user-part, but it won't match usernames by domain anymore. (i.e. it doesn't treat sip:domain as sip:domain at domain anymore)
> 
> (3) there was some freaky logic going on in get_msg_text, I had to rewrite it to make it make sense.
> 
> (4) in the reqresp parser there were lots of if (params) inside a big if (params) block. I scrapped the useless if's.
> 
> 
> This addresses bug ASTERISK-18389.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18389
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 341249 
>   /trunk/channels/sip/reqresp_parser.c 341249 
> 
> Diff: https://reviewboard.asterisk.org/r/1533/diff
> 
> 
> Testing
> -------
> 
> It compiles.
> 
> 
> Thanks,
> 
> wdoekes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111020/08c18b93/attachment.htm>


More information about the asterisk-dev mailing list