[asterisk-dev] [Code Review] 3349: Implement RFC-3966 TEL URI incoming INVITE

Fri Apr 11 08:44:00 CDT 2014

> On March 22, 2014, 10:39 a.m., Olle E Johansson wrote:
> > I don't see what happens with the phone-context argument. Shouldn't we pass that on as a channel variable?
> 
> Geert Van Pamel wrote:
>     We return this into the hostport.
> 
> Geert Van Pamel wrote:
>     According to RFC 3966 phone-context is either a domain-name, or (part of) an international telephone number (indicated with +prefix).
>     It is used by a gateway to know how to dial the "local" number... the local number must be unique within its context...
> 
> Olle E Johansson wrote:
>     So it ends up in the SIPDOMAIN variable in the dial plan? It has to be reachable in the dial plan somehow.
> 
> Geert Van Pamel wrote:
>     The variable ${SIPDOMAIN} contains the local IP address of the Asterisk server.
>     The userinfo arrives in ${CALLERID} and is displayed on the display of the called device, and arrives in the CDR file.
>     Actually I do not know into which variable the incoming hostport info is copied to?
>     Could somebody else answer this question?
> 
> Olle E Johansson wrote:
>     If I place a normal call to sip:geert at example.com to my Asterisk server. "geert" will be the extension I'm looking for, "example.com" ends up in SIPDOMAIN. It's not the local IP address, it's the domain/host part of the request URI in the INVITE.
>     
>     I would prefer if phone context ended up in TELPHONECONTEXT so I could use it the same way as SIPDOMAIN in the dial plan. It should not end up in SIPDOMAIN as it is not a SIP uri. That way an extension in a local context can be routed differently than an extension in a global context.
> 
> Olle E Johansson wrote:
>     Just to make sure it's documented: The phone-context should NOT be matched to a context in the dial plan. From a security standpoint, that would be horrific. We need the information to be able to route correctly in the dial plan, but no automatic selection. (I am not suggesting you where going down that path, Geert. Just wanted to close that way of thinking since the word "context" could lead there.)
> 
> Geert Van Pamel wrote:
>     So I understand that Olle proposes to create a new variable ${TELPHONECONTEXT} that contains the TEL URI phone-context which can be either the global number, or a global number prefix, or the related routing domain or any other unique routing identifier, or would be empty in case there is just a local number (as specified in RFC 3966).
>     
>     Currently this variable is not available yet in Asterisk. In the dialplan treating incoming calls currently I do not use nor do not need this information, as the local number in ${CALLERID} is sufficient (for the time being)... Still this phone context identifier could be needed for subsequent outgoing calls (return calls, callbacks, etc.). 
>     
>     I agree that ${SIPDOMAIN} will remain reserved for SIP invites, and is untouched for TEL URI invites.
>     
>     I perfectly understand that this TEL URI context has nothing to do with dialplan context.
>     
>     Who could us further advise how to create the new variable ${TELPHONECONTEXT} ?
> 
> Olle E Johansson wrote:
>     Just grep for SIPDOMAIN in the chan_sip source code :-)
> 
> Matt Jordan wrote:
>     So I'd hate for this patch to stall out on the channel variable - which would be a very helpful addition but is not strictly necessary for Geert's functionality.
>     
>     Olle - what do you think if this goes in as is, and we add the TELPHONECONTEXT channel variable in a subsequent patch? It should be relatively trivial, and I don't mind helping Geert with that work (so long as Geert is willing to test said patch, as I don't have anything that emulates tel URIs handy).
>     
>     Geert - what do you think?
> 
> Geert Van Pamel wrote:
>     Matt, I agree with you... the variable ${TELPHONECONTEXT} is indeed not needed for TEL URI INVITE to work. Actually, I a uncertain how to implement this variable. If anybody else can code the variable later, I am willing to test it...
> 
> Matt Jordan wrote:
>     Geert - I can take a look at adding that variable in a subsequent patch. I'll definitely need you to test it!
>     
>     I'll confirm with Olle that he's okay with this - if so, we'll get this committed.
> 
> wdoekes wrote:
>     That's a negative:
>     
>     14:26 < wdoekes> whether we can ship it without the TELURICONTEXT var
>     14:26 <@oej> Did they get the channel variable?
>     14:26 < wdoekes> for another rainy day, it was said
>     14:28 <@oej> Why is that? Only sending incomplete information to the dialplan is not a good implementation.
>     14:28 <@oej> It's a simple change, nothing much to discuss. Many examples in the source code of chan_sip....
>     14:29 <@oej> I guess I should have discussed this with Matt last week.
>     14:30 < wdoekes> not sure it's that trivial, since the info has to get out of the parser functions: a bit of refactoring
>     14:31 <@oej> It can't be that hard. Ignoring it would be bad and an issue report is on the mail in a very nearby future
>     14:31 <@oej> The phone context is an important part of the URI
>
> 
> Matt Jordan wrote:
>     Well... the hardest part about this right now is actually reviewing the channel variable. I (mostly) have a patch that adds it. We have two options:
>     
>     (1) I can post a review that includes the variable as well as this entire patch
>     (2) We can commit this, then I can post a review that only includes the variable
>     
>     Either way, the variable will go in with this functionality into trunk - it's really just an order of operations problem.
> 
> Olle E Johansson wrote:
>     If you promise this is fixed before release, I'm fine.

I promise! :-)

I'll make sure I put it up for review immediately after committing.

- Matt

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

On March 22, 2014, 8:08 a.m., Geert Van Pamel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3349/
> -----------------------------------------------------------
> 
> (Updated March 22, 2014, 8:08 a.m.)
> 
> 
> Review request for Asterisk Developers, Corey Farrell, lmadensen, Matt Jordan, and wdoekes.
> 
> 
> Bugs: ASTERISK-17179
>     https://issues.asterisk.org/jira/browse/ASTERISK-17179
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Implements RFC-3966 TEL URI incoming INVITE.
> 
> See https://issues.asterisk.org/jira/browse/ASTERISK-17179 for a description of the original isssue.
> 
> I have been patching all versions since Asterisk 1.6. I would like to include the code into the main trunk for version 13.
> 
> Previously Asterisk was failing with error on incoming IMS call:
> 
> Nov 13 17:52:05 NOTICE[27459]: chan_sip.c:6973 check_user_full: From address missing 'sip:', using it anyway
> 
> Nov 13 17:52:05 WARNING[27459]: chan_sip.c:6525 get_destination: Huh? Not a SIP header (tel:0987654321;phone-context=+32987654321)?
> 
> Reason: tel: protocol was not recognized.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/sip/reqresp_parser.c 410429 
>   /trunk/channels/chan_sip.c 410429 
> 
> Diff: https://reviewboard.asterisk.org/r/3349/diff/
> 
> 
> Testing
> -------
> 
> Executed an incoming TEL URI INVITE connection.
> CLI was present on the display and in the CDR file.
> No errors on SIP debug output.
> 
> 
> File Attachments
> ----------------
> 
> RFC-3966 tel URI patch
>   https://reviewboard.asterisk.org/media/uploaded/files/2014/03/13/cad7a996-88c1-47fe-a2a9-cc6987af3b75__rfc-3966-tel-uri-patch-diff.txt
> 
> 
> Thanks,
> 
> Geert Van Pamel
> 
>

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