[Home]

Summary:ASTERISK-07019: INVITE parsing fails when options exist in URI before @ sign. Fails carrier interop testing with Sonus GWs.
Reporter:Scott Keagy (skeagy)Labels:
Date Opened:2006-05-23 00:14:04Date Closed:2006-06-26 14:22:33
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Interoperability
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) Sonus_call_failure.txt
Description:See description I posted at:
http://forums.digium.com/viewtopic.php?t=6781

Basically, INVITE parsing fails for URIs like this:
sip:2028210181;npdi=yes@172.16.0.10:5060;dtg=SIP;user=phone SIP/2.0

Result is domain erroneously set to called party number, and extension set to 's' instead of the called party number.

****** ADDITIONAL INFORMATION ******

Sorry I don't have a patch to submit, I'm not a coder. But I gave pretty good guidelines to the exact part of code with a problem, and suggestion to fix it:
http://forums.digium.com/viewtopic.php?t=6781
Comments:By: Scott Keagy (skeagy) 2006-05-23 18:53:59

Folks, we're talking 6 new lines of code (excluding comments) and 3 existing lines commented out, and a single new variable declared. Simple fix, to solve a painful problem.

I don't know how to use SVN, I don't have a waiver on file. I release all proprietary interest in this code and welcome somebody to implement this diff from chan_sip.c in asterisk 1.2.7.1:

[ast@asterisk1 upload]# diff chan_sip.c chan_sip.c.orig
6525c6525
<       char tmp[256] = "", *uri, *a, *b;
---
>       char tmp[256] = "", *uri, *a;
6561,6563c6561,6563
< /*    if ((a = strchr(uri, ';'))) {   */
< /*            *a = '\0';              */
< /*    }                               */
---
>       if ((a = strchr(uri, ';'))) {
>               *a = '\0';
>       }
6569,6578d6568
<
<               /* skip any options that follow domain part of URI */
<               if ((b = strchr(a, ';'))) {
<                       *b = '\0';
<               }
<               /* skip any options that follow user part of URI */
<               if ((b = strchr(uri, ';'))) {
<                       *b = '\0';
<               }
<


In case this diff format is too ugly, here's the straight code from that piece:
       /* Skip any options */
/*      if ((a = strchr(uri, ';'))) {   */
/*              *a = '\0';              */
/*      }                               */

       /* Get the target domain */
       if ((a = strchr(uri, '@'))) {
               *a = '\0';
               a++;

               /* skip any options that follow domain part of URI */
               if ((b = strchr(a, ';'))) {
                       *b = '\0';
               }
               /* skip any options that follow user part of URI */
               if ((b = strchr(uri, ';'))) {
                       *b = '\0';
               }


By: Andrey S Pankov (casper) 2006-05-23 20:12:10

skeagy: The right way for diff is to use its "unified" format (diff -u).
And you need to have your disclaimer on file with Digium IMO.


By: Andrey S Pankov (casper) 2006-05-23 20:35:17

This is not a bug, this is a feature request.

And this is not as simple as it may look from the first view...

For references:
RFC3261 "SIP: Session Initiation Protocol" Section 19.1.6 "Relating SIP URIs and tel URLs"
RFC2806 "URLs for Telephone Calls"
By: Scott Keagy (skeagy) 2006-05-23 23:12:24

Thanks for feedback. I'll do the waiver. Stuff I posted with the diff turned out not to fix the problem... the URI coming into get_destination has already been stripped of everything after the first semicolon.

So my simple work-around is this... after uri field is set to 's', I perform a check that p->exten is not null, and then make uri = p->exten. I don't bother about the erroneous setting of domain.

Only 1 line of code inserted, but it enables me to focus on passing interop test & getting back to business. Hopefully someone else can address this as a real feature enhancement (i.e. proper handling of sip & tel URIs).
By: Olle Johansson (oej) 2006-05-24 07:10:29

Does it work with svn trunk?
By: Kevin P. Fleming (kpfleming) 2006-05-24 13:57:39

Can you post an RFC reference that says this format is even allowed? On the surface it looks to be completely non-conforming, or at least an 'extension' that we can't be expected to have support for without some way to document it and test it.
By: Kevin P. Fleming (kpfleming) 2006-05-24 15:40:29

Reclassifying as 'MINOR' since this is not documented behavior.
By: Andrey S Pankov (casper) 2006-05-25 10:33:28

kpfleming: please look at the note 0046784. I already added references there...
By: Kevin P. Fleming (kpfleming) 2006-05-25 10:36:47

Sorry, I don't have time to read all of RFC3261 again to try to verify that there is (or is not) language that supports this type of URI. It would be much better if someone who actually has an interest in this thing being supported could provide a direct citation that shows it being acceptable.
By: Scott Keagy (skeagy) 2006-05-25 10:58:40

Here's RFC-track document... currently still IETF draft:
http://www.ietf.org/internet-drafts/draft-ietf-iptel-tel-np-10.txt

I just skimmed through relevant part of SIP RFC 3261 (section 25), and the syntax that Sonus uses violates RFC 3261 and is not consistent with latest number portability draft (because Sonus uses sip: instead of tel:, and they use ;npdi=yes instead of just ;npdi;rn=<whatever>. Nevertheless, they have major market share amongst VoIP telcos. If you want to peer in U.S. with Level3, MCI, AT&T, or XO, you have to support the Sonus weirdness. That means add support to Asterisk or buy an expensive session border controller for peering and then just use Asterisk as an app on the back end.

I'll update with more findings from my official interop tests with a large telco, but it looks like my 1-line fix in chan_sip.c makes Asterisk route calls correctly when the weird parameter is present:

if (p->exten) uri = p->exten;    /* undo erroneous assignment of 's' to uri */


did enough of the job to pass all tests from my perspective (still need to await their approval after reviewing SIP traces).


By: Kevin P. Fleming (kpfleming) 2006-05-25 12:43:59

OK, a few comments:

First, since this is only a draft, it's a bit early to be demanding that people implement it, no matter how large the telco and/or equipment manufacturer may be.

Second, this seriously changes the way that URI parsing must be done and it's not a good idea to be doing that in a release branch. That means we may choose not do to this at all for 1.2.x, but only for the upcoming 1.4 release.

Third, I will look over this issue again in a few days and talk about it with our other primary chan_sip developer to see what his opinions are. We have to keep in mind that adding complexity and execution time for all SIP URI parsing just because a small few need this is not always a good choice :-)
By: Olle Johansson (oej) 2006-06-05 03:34:33

Since this is only a draft at this stage, I fixed this in svn trunk for 1.4, but not for 1.2. RFC3261 allows for ; in the userinfo part, and we might as well strip any of those options out. Please test svn trunk now.
By: Brett Nemeroff (brettnem) 2006-06-05 11:41:41

Tested with Asterisk SVN-trunk-r32281 and it's still not working. Note I have both npdi and rn in my options:
INVITE sip:4327891910;rn=4327891097;npdi=yes@207.90.232.10:5060;dtg=UTEXHSTNSIPORIG02;user=phone SIP/2.0

...

Looking for s in default (domain 4327891910)


I have not tried the patches listed above but this is in response to oej's comment that it's fixed in svn trunk now.
By: Olle Johansson (oej) 2006-06-06 03:21:25

brettnem: Thanks for the feedback, will go back and look into this once more.
By: Olle Johansson (oej) 2006-06-06 03:46:16

Please check that you really have the latest svn trunk, not anything based on 1.4. On my server it does remove userinfo-options and domain options separately now.
By: Olle Johansson (oej) 2006-06-06 03:46:38

Sorry, of course I meant "not anything based on 1.2" :-)
By: Brett Nemeroff (brettnem) 2006-06-06 07:07:29

oej,
I really don't know how to tell what version the code is based on. However show version does report:
Asterisk SVN-trunk-r32281

I svn co the code just after you said it was fixed. Is there some other way I should be doing this?
By: Olle Johansson (oej) 2006-06-06 10:57:07

run "svn update" inside the source code directory, run "make clean", then make and make install. Thanks!
By: Brett Nemeroff (brettnem) 2006-06-06 14:25:03

Did it again, still not working:
myhost*CLI> show version
Asterisk SVN-trunk-r32281 built by root @ myhost on a i686 running Linux on 2006-06-05 15:06:51 UTC
By: Olle Johansson (oej) 2006-06-06 15:40:56

Please add sip debug output showing it not working. Thanks

set debug to 4
By: Brett Nemeroff (brettnem) 2006-06-06 16:12:45

oej,
I'm hoping this is what you want.. let me know..
By: Andrey S Pankov (casper) 2006-06-07 15:28:55

brettnem: there is no _debug_ otput in your trace, only _verbose_...
By: Serge Vecher (serge-v) 2006-06-07 15:56:18

brettnem: Please make sure you have the following line in your logger.conf:
console => notice,warning,error,debug

restart logger or asterisk, type set debug 4 and try again.
By: Serge Vecher (serge-v) 2006-06-15 09:30:48

brettnem: need your response here
By: Olle Johansson (oej) 2006-06-26 14:22:32

No answer.