[prev in list] [next in list] [prev in thread] [next in thread] 

List:       asterisk-dev
Subject:    Re: [asterisk-dev] [Code Review] 3349: Implement RFC-3966 TEL URI incoming INVITE
From:       "Matt Jordan" <reviewboard () asterisk ! org>
Date:       2014-03-28 17:37:29
Message-ID: 20140328173729.26055.88644 () sonic ! digium ! api
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


> 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@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...

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.


- 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
> 
> 


[Attachment #5 (text/html)]

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;">  <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://reviewboard.asterisk.org/r/3349/">https://reviewboard.asterisk.org/r/3349/</a>
  </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <p style="margin-top: 0;">On March 22nd, 2014, 10:39 a.m. CDT, <b>Olle E \
Johansson</b> wrote:</p>  <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;">  <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">I don&#39;t see what happens with the phone-context argument. \
Shouldn&#39;t we pass that on as a channel variable? </pre>  </blockquote>




 <p>On March 22nd, 2014, 3:23 p.m. CDT, <b>Geert Van Pamel</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">We return this into the \
hostport.</pre>  </blockquote>





 <p>On March 22nd, 2014, 4:01 p.m. CDT, <b>Geert Van Pamel</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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 \
&quot;local&quot; number... the local number must be unique within its \
context...</pre>  </blockquote>





 <p>On March 23rd, 2014, noon CDT, <b>Olle E Johansson</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So it ends up in the \
SIPDOMAIN variable in the dial plan? It has to be reachable in the dial plan somehow. \
</pre>  </blockquote>





 <p>On March 23rd, 2014, 4:12 p.m. CDT, <b>Geert Van Pamel</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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?</pre>
 </blockquote>





 <p>On March 24th, 2014, 2:05 a.m. CDT, <b>Olle E Johansson</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">If I place a normal call \
to sip:geert@example.com to my Asterisk server. &quot;geert&quot; will be the \
extension I&#39;m looking for, &quot;example.com&quot; ends up in SIPDOMAIN. It&#39;s \
not the local IP address, it&#39;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.</pre>  </blockquote>





 <p>On March 24th, 2014, 4:24 a.m. CDT, <b>Olle E Johansson</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Just to make sure \
it&#39;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 &quot;context&quot; could lead there.)</pre>  </blockquote>





 <p>On March 24th, 2014, 5:32 a.m. CDT, <b>Geert Van Pamel</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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} ?</pre>
 </blockquote>





 <p>On March 24th, 2014, 5:39 a.m. CDT, <b>Olle E Johansson</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Just grep for SIPDOMAIN \
in the chan_sip source code :-)</pre>  </blockquote>





 <p>On March 27th, 2014, 11:14 p.m. CDT, <b>Matt Jordan</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So I&#39;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&#39;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&#39;t mind helping Geert with that work (so long as Geert is willing to test said \
patch, as I don&#39;t have anything that emulates tel URIs handy).

Geert - what do you think?</pre>
 </blockquote>





 <p>On March 28th, 2014, 11:53 a.m. CDT, <b>Geert Van Pamel</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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...</pre>  </blockquote>








</blockquote>

<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Geert - I can take a \
look at adding that variable in a subsequent patch. I&#39;ll definitely need you to \
test it!

I&#39;ll confirm with Olle that he&#39;s okay with this - if so, we&#39;ll get this \
committed.</pre> <br />










<p>- Matt</p>


<br />
<p>On March 22nd, 2014, 8:08 a.m. CDT, Geert Van Pamel wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;">  <tr>
  <td>

<div>Review request for Asterisk Developers, Corey Farrell, lmadensen, Matt Jordan, \
and wdoekes.</div> <div>By Geert Van Pamel.</div>


<p style="color: grey;"><i>Updated March 22, 2014, 8:08 a.m.</i></p>







<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="https://issues.asterisk.org/jira/browse/ASTERISK-17179">ASTERISK-17179</a>


</div>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">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 \
&#39;sip:&#39;, 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.</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Executed an incoming TEL URI INVITE connection. CLI was present on the \
display and in the CDR file. No errors on SIP debug output.
</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>/trunk/channels/sip/reqresp_parser.c <span style="color: \
grey">(410429)</span></li>

 <li>/trunk/channels/chan_sip.c <span style="color: grey">(410429)</span></li>

</ul>

<p><a href="https://reviewboard.asterisk.org/r/3349/diff/" style="margin-left: \
3em;">View Diff</a></p>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments \
</h1>

<ul>

 <li><a href="https://reviewboard.asterisk.org/media/uploaded/files/2014/03/13/cad7a996-88c1-47fe-a2a9-cc6987af3b75__rfc-3966-tel-uri-patch-diff.txt">RFC-3966 \
tel URI patch</a></li>

</ul>





  </td>
 </tr>
</table>








  </div>
 </body>
</html>



-- 
_____________________________________________________________________
-- 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

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic