[asterisk-dev] [Code Review] 3995: res_pjsip_endpoint_identifier_ip: Can't parse identify with match value containing CIDR

[Mark Michelson]

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

Ship it!


A couple of minor nitpicks, but other than that it looks good to go.


/branches/12/res/res_pjsip_endpoint_identifier_ip.c
<https://reviewboard.asterisk.org/r/3995/#comment23810>

    Since you switched from the goto usage from your first diff, you can go back to not initializing addrs.



/branches/12/res/res_pjsip_endpoint_identifier_ip.c
<https://reviewboard.asterisk.org/r/3995/#comment23809>

    Print current_string instead of var->value here.


- Mark Michelson


On Sept. 17, 2014, 10:29 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3995/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2014, 10:29 p.m.)
> 
> 
> Review request for Asterisk Developers and Joshua Colp.
> 
> 
> Bugs: ASTERISK-24290
>     https://issues.asterisk.org/jira/browse/ASTERISK-24290
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Using a value such as '10.24.0.0/16' would fail to match because ast_sockaddr_resolve can only parse the following formats:
> 
>  * hostname:port
>  * host.example.com:port
>  * a.b.c.d
>  * a.b.c.d:port
>  * a:b:c:...:d
>  * [a:b:c:...:d]
>  * [a:b:c:...:d]:port
> 
> When the format doesn't match one of these, the function fails and we bail.
> 
> To get around this, I simply checked for the presence of a '/' in the identify string and used ast_append_ha directly with the address if it was present.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_pjsip_endpoint_identifier_ip.c 423062 
> 
> Diff: https://reviewboard.asterisk.org/r/3995/diff/
> 
> 
> Testing
> -------
> 
> Used CLI command 'pjsip show endpoint 1603' with an endpoint that had the following identifier:
> 
> [1603]
> type=identify
> match=10.24.18.13/16
> endpoint=1603
> 
> 
> Before, the address would fail to parse and the command would show no identifier
> After, the address would parse correctly and show '10.24.0.0/16' for the identifier as seen in:
> 
>  Endpoint:  1603/1603                                            Not in use    0 of inf
>         Aor:  1603                                               5
>       Contact:  1603/sip:1603 at 10.24.18.13:5060;ob                Unknown               nan
>    Identify:  10.24.0.0/16
> 
> I tried a few other things, such as not using a CIDR and using a hostname to verify that there wasn't any obvious deviation in behavior introduced by the patch.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140917/4b7670ce/attachment-0001.html>