> 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@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 > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
