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

[Jonathan Rose]

> On Sept. 16, 2014, 1:16 p.m., Scott Griepentrog wrote:
> > /branches/12/res/res_pjsip_endpoint_identifier_ip.c, lines 162-166
> > <https://reviewboard.asterisk.org/r/3995/diff/1/?file=67358#file67358line162>
> >
> >     The implementation of ast_append_ha supports comma separated values, but pjsip.conf.sample doesn't clearly indicate support nor lack of support for commas, although it does show examples of multiple match lines to accomplish the same thing.
> >     
> >     Assuming that we are expressly not going to support something like this:
> >     
> >     match=sip.example.com,1.2.3.4/10
> >     
> >     Then this code is fine, otherwise would need to be reworked to parse the , values out first.
> >

Personally, I feel inclined to either not allow commas in the case of the example you mentioned or else perform comma separation while parsing the config rather than allow ast_apply_ha handle it. Right now if that were used I think it would fail since ast_apply_ha doesn't do host name matching... and otherwise we'd have an issue when people try to use comma separation with anything other than a collection of host addresses with at least one of them containing the CIDR pattern.

I'll defer that decision to someone else though. Neither is particularly difficult to implement, I'm just not sure which is preferred.


- Jonathan


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


On Sept. 15, 2014, 1:52 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3995/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2014, 1:52 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/03453a80/attachment-0001.html>