[asterisk-dev] [Code Review] SIP from-header is parsed twice

[Nick Lewis]

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