[asterisk-dev] [Code Review] Explicit context set in SIP peer overridden by default domain context

[Nick Lewis]

> On 2010-04-12 09:35:33, Mark Michelson wrote:
> > As far as code integrity and falling within the Asterisk coding guidelines goes, this is very well done. Great work!
> > 
> > The only thing is that I don't really feel comfortable greenlighting a behavior change such as this one without some sort of discussion and/or consensus being reached first. It's mainly due to the fact that I'm not one who uses SIP domains. I agree with this change myself, for the record. Could someone else please weigh in on this? Olle, perhaps?
> 
> pprindeville wrote:
>     I've asked Olle to review it as well, but so far I've not had a response.
>
> 
> Nick Lewis wrote:
>     Although the current behaviour of domain-lists is detrimental to peers of type=user (e.g. registered internal phones) it may be expected behaviour for peers of type=peer such as connections to service providers. Therefore to remove domain-contexts from being applied to all specified peers may be problematic in multi-tenancy applications
>     
>     One way around this would be to add domain-contexts to specific peer definitions as well as to global definitions. The domain-contexts in the global definitions would apply only when there was no peer match but domain-contexts (domain=<sip-to-domain>,<context>) could be added to peer definitions to provide domain-contexts on a per peer basis to those peers that require it.
>     
>     On a broader point I question the wisdom of having any domain-contexts in chan_sip.c, irrespective of whether they be peer specific or global. domain-contexts are clearly a form of routing and (as oej frequently points out) all routing should be done in the dial-plan. An alternative approach is to advertise SIPTODOMAIN as a variable in the dialplan and in extensions.conf do something like
>     
>     [guest-domain-contexts]
>     exten => _.,1,gotoif($[${SIPTODOMAIN} = "firsttenant.com"]?firsttenant-guest,${EXTEN},1)
>     exten => _.,n,gotoif(${${SIPTODOMAIN} = "secondtenant.com"]?secondtenant-guest,${EXTEN},1)
>     
>     [from-itsp-domain-contexts]
>     exten => _.,1,gotoif($[${SIPTODOMAIN} == "firsttenant.com"]?firsttenant,${EXTEN},1)
>     exten => _.,n,gotoif($[${SIPTODOMAIN} == "secondtenant.com"]?secondtenant,${EXTEN},1)
>     
>

Actually not SIPTODOMAIN but SIPREQUESTURIDOMAIN though it may become SIPTODOMAIN once to-header based DID is supported. Perhaps EXTENDOMAIN would be better. Anyway this is future stuff. To get the current stuff working as oej intended I suggest a flag is set for a peer specific context configuration and this suppresses the domain-context overwrite


- Nick


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


On 2010-04-09 14:41:36, pprindeville wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/565/
> -----------------------------------------------------------
> 
> (Updated 2010-04-09 14:41:36)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Having inbound SIP 'guest' calls associated to a default context is
> handy, especially when multi-tenancy or hosting multiple domains is
> being done.
> 
> However, the code as written will clobber an explicitly configured
> context associated with a peer if a domain-list is configured.
> 
> This is counter-intuitive, since (a) explicit configuration should
> always trump default or implicit config, and (b) puts all internal
> domain-bound SIP handsets in the same context as guest callers, making
> it hard to apply a restricted dialplan to guest callers.
> 
> 
> This addresses bug 17040.
>     https://issues.asterisk.org/view.php?id=17040
> 
> 
> Diffs
> -----
> 
>   branches/1.6.2/channels/chan_sip.c 256425 
> 
> Diff: https://reviewboard.asterisk.org/r/565/diff
> 
> 
> Testing
> -------
> 
> Running patched 1.6.2.6 on our production network.  Seems to work fine.
> 
> 
> Thanks,
> 
> pprindeville
> 
>