[asterisk-dev] [Code Review] SIP user fields are crazy. Repeat extension searches if they all fail and semicolons are obfuscating the extension in the uri.

Thu May 12 14:21:39 CDT 2011

Alright, so then we've reached a consensus that we are basically going
to try and specifically resolve this issue simply by having an option to
split from the semi-colon only on channels specifically defined to be
handled that way.

In that case, this is basically the same as
https://reviewboard.asterisk.org/r/1188
revision 2.

That leaves a couple things needing to be addressed:

The discussion on that particular revision was as follows:

        -----------------------------------------------------------------------------------------
        Posted 3 weeks, 2 days ago (April 18th, 2011, 3:16 p.m.)
        Would it make sense to make this something more configurable? The ABNF for a SIP-URI is:
        SIP-URI          =  "sip:" [ userinfo ] hostport
                            uri-parameters [ headers ]
        userinfo         =  ( user / telephone-subscriber ) [ ":" password ] "@"
        user             =  1*( unreserved / escaped / user-unreserved )
        user-unreserved  =  "&" / "=" / "+" / "$" / "," / ";" / "?" / "/"

        So down the road, it is entirely possible that someone does something weird like INVITE sip:user?foo=bar&bar=baz and we would need another workaround option. 
        We could make the option something like uri_exten_terminator=?;&/ or something since you are matching with strpbrk anyway. Of course, then someone might say 
        that we need to have an option to do regex matches on URIs. Then someone else will point out the limitations of regexes. :-p But, it seems like a cheap and 
        easy change that doesn't make things any more confusing. Others may disagree
             1. 
                jrose 3 weeks, 2 days ago (April 18th, 2011, 3:40 p.m.) 
                        I suppose it wouldn't be hard at all to have an option like...

                        uri_break_characters=;:/
                        instead.  Especially since the method this uses for terminating is strpbrk against ";" on the uri string.  I wouldn't have to do a whole lot to change it.

                        I really don't know for certain though what the best option for this is.  I'm still pretty green when it comes to SIP and the various scenarios we might encounter with it.
             1. 
                jrose 3 weeks, 2 days ago (April 18th, 2011, 3:41 p.m.) 
                        And rereading what you just wrote, that's pretty much exactly what you said.  Oi, I'm a little tired.
             1. 
                Olle E Johansson 3 weeks, 2 days ago (April 19th, 2011,
                1:24 a.m.) 
                        I think this is a bug. The URI username options - which this seems to be about - should be removed from the actual extension. Maybe be put in a variable for later processing, like SIPFULLUSERNAME (username AND options) or SIPUSERNAMEOPTIONS - just the options. That way, I could restore them on the outbound call leg if needed.

                        Please test with 1.4 to compare behaviours.

                        Terry - the Username options is not part of your ABNF because it was added in a later RFC.
             1. 
                Terry Wilson 3 weeks, 2 days ago (April 19th, 2011, 8:08
                a.m.) 
                        Agreed. We never should have been matching these to an extension in the first place; it is a bug, so we probably don't even need to make this optional. We also shouldn't just completely throw them away, either. We have a function in channels/sip/reqresp_parser.c named parse_uri_full() that seems to break everything up into its constituent parts. I don't think we've ever used it though. If we did parse it all out and store it, it would make it easily available for getting/setting via dialplan variables, functions, etc. For reference, RFC 3261 19.1.1

        -----------------------------------------------------------------------------------------------

With that in mind, should I still be bothering to preserve these user
parameters, and should I bother to separate on any additional
characters?  After looking over the RFCs, I was heavily under the
impression that only a semicolon would be used to distinguish the
beginning of a user parameter, all of which would be at the end of the
actual user.  This was all in a different section of the RFCs though,
pertaining to the conversion of TEL URIs to SIP URIs.

On Thu, 2011-05-12 at 13:31 -0500, David Vossel wrote:
> 
> 
> ----- Original Message -----
> > From: "Leif Madsen" <leif.madsen at asteriskdocs.org>
> > To: asterisk-dev at lists.digium.com
> > Sent: Thursday, May 12, 2011 1:18:02 PM
> > Subject: Re: [asterisk-dev] [Code Review] SIP user fields are crazy. Repeat extension searches if they all fail and
> > semicolons are obfuscating the extension in the uri.
> > On 11-05-12 01:52 PM, David Vossel wrote:
> > > For those people who have devices that do this to the user field and
> > > they don't want it there, lets just make a sip.conf option that
> > > strips those values out entirely out of every URI so their devices
> > > work. No storing of the values in channel variables or anything like
> > > that to complicated it. Just strip them off and let pattern matching
> > > do its thing without them. If my understanding is correct, that is
> > > all people really want that are having interoperability issues with
> > > these user field options.
> > >
> > > I'd imagine the option would look like this.
> > >
> > > strip_uri_user_options = yes ; When this option is enabled any parts
> > > of the user field of a URI delimited by a ';' will be stripped off
> > > before extensions or peer pattern matching occurs. This option is
> > > purely for interoperability purposes for devices that put options in
> > > the user field that are not supposed to be used during pattern
> > > matching.
> > >
> > > If someone doesn't what those undefined fields stripped off their
> > > user fields, then they don't enable the option and have to do
> > > dialplan magic to get the extension to match correctly.
> > >
> > > Does this work for everyone? I know there is more to discuss here,
> > > but this approach seems to have merit regardless of what other
> > > changes in this area may be introduced in the future.
> > 
> > I could get behind this idea. It should be disabled by default.
> > 
> > Does this go into 1.8 as a "feature that fixes a bug"?
> > 
> > Leif.
> > 
> > --
> 
> Yes, this is an interop issue.  Anything that makes us work with stuff at this sort of level should be considered a fix in my opinion.
> 
> I agree with the option being off by default as well since this is not a standards based thing.
> 