[asterisk-dev] [Code Review] Truncate host:port when checking SIP domains and make the URI parsing API more explicit about returning host:port and not a domain

Simon Perreault reviewboard at asterisk.org
Tue Nov 8 19:56:38 CST 2011


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



/branches/1.8/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1574/#comment8923>

    This will fail with IPv6 address literals.
    
    e.g. "[2001:db8::1]:1234"
    
    You need to check for brackets. (Unless I'm mistaken about this function's expected input range, in which case this should be documented in the doxygen comments.)
    
    Isn't there code in netsock2.c that already does this?


- Simon


On Nov. 8, 2011, 7:41 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1574/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2011, 7:41 p.m.)
> 
> 
> Review request for Asterisk Developers and wdoekes.
> 
> 
> Summary
> -------
> 
> In reviewing https://reviewboard.asterisk.org/r/1533/ I noticed that going from 1.4 to 1.8, setting domain=x.x.x.x stopped matching when receiving a request to x.x.x.x. The reason is that the parse_uri function returns host:port in what was labeled the "domain" variable in 1.8 when in 1.4 the parsing was done in each individual function and the port truncated manually. For example, peer->fromdomain could be set to a host:port, even though it is specifically split into peer->fromdomain and peer->fromport other places in the code.
> 
> We can't change the parsing function because sometimes we actually use the host:port form of the result (as in when passing it to ast_sockaddr_resolve_first), so we need to do the truncation manually when appropriate. This patch does two things: 1) it renames the "domain" variables in the API functions to "hostport" to more accurately reflect what we are really returning and 2) implements a truncate_hostport function and uses it where appropriate.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 343673 
>   /branches/1.8/channels/sip/include/reqresp_parser.h 343673 
>   /branches/1.8/channels/sip/include/sip.h 343673 
>   /branches/1.8/channels/sip/reqresp_parser.c 343673 
> 
> Diff: https://reviewboard.asterisk.org/r/1574/diff
> 
> 
> Testing
> -------
> 
> Ran the SIP unit tests and SIP-related testsuite tests and saw no difference in behavior. Set domain=127.0.0.1 and verified register requests started working.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111109/17d6d8e7/attachment.htm>


More information about the asterisk-dev mailing list