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

George Joseph reviewboard at asterisk.org
Wed Sep 17 15:54:13 CDT 2014



> On Sept. 16, 2014, 12: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.
> >
> 
> Jonathan Rose wrote:
>     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.

Unless it's significantly more work, I think the case of mixed formats has to be handled which means handling the comma separation while parsing the config.  From a user perspective, it'd be weird to have a restriction that only formats of the same type can be used in a comma separated list.


- George


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


On Sept. 15, 2014, 12: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, 12: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/f36e74c5/attachment.html>


More information about the asterisk-dev mailing list