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

List:       asterisk-dev
Subject:    Re: [asterisk-dev] [Code Review] SIP from-header is parsed twice
From:       "Nick Lewis" <Nick.Lewis () atltelecom ! com>
Date:       2010-06-30 8:22:26
Message-ID: 20100630082226.21291.42151 () hotblack ! digium ! com
[Download RAW message or body]



> On 2010-06-30 02:22:21, richardf wrote:
> > trunk/channels/chan_sip.c, lines 13849-13856
> > <https://reviewboard.asterisk.org/r/701/diff/3/?file=10973#file10973line13849>
> > 
> > I think now is a good time to fix this aspect of RFC non compliance. I would \
> > suggest something like username = NULL; I can't think of any way what fixing this \
> > could cause backward compatibility problems. I have tested this patch with this \
> > section setting username to NULL and had no problems.

I moved this from its previous location without really considering it. It does not \
really seem to do all that it says in the comment. Rather than considering a missing \
@host as a username-only uri for backward compatibility it seems instead to treat it \
as a uri with identical username and domain. To work as described I think it should \
also do p->fromdomain = NULL or set the string length to zero. So not only is it \
non-compliant but it also does not work.


I agree that this should go away am happy to remove it if there is a consensus \
(mm/dv). However setting username=NULL may not be needed as most conditions look for \
zero length strings rather than null string pointers 


On 2010-06-30 02:22:21, Nick Lewis wrote:
> > I have tested this patch and apart from the above comment I think this is great \
> > and ready to go.

Thanks for testing this. I hope it is close to a shipit/commit


- Nick


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


On 2010-06-22 08:57:10, Nick Lewis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/701/
> -----------------------------------------------------------
> 
> (Updated 2010-06-22 08:57:10)
> 
> 
> Review request for Asterisk Developers and David Vossel.
> 
> 
> Summary
> -------
> 
> The handlers in chan_sip.c for invite and subscription requests parse the \
> from-header twice. The first time is in check_user_full and the second time is in \
> get_destination. This change removes the parsing from these functions and does it \
> beforehand (with an abnf compliant parser) 
> 
> This addresses bug 16897.
> https://issues.asterisk.org/view.php?id=16897
> 
> 
> Diffs
> -----
> 
> trunk/channels/chan_sip.c 268968 
> trunk/channels/sip/reqresp_parser.c 268968 
> 
> Diff: https://reviewboard.asterisk.org/r/701/diff
> 
> 
> Testing
> -------
> 
> Compiles, runs, passes associated unit test. CID field appears correct in sip show \
> channel  
> 
> Thanks,
> 
> Nick
> 
> 


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