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

[David Vossel]

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


Thanks again for your contributions Nick!  I've looked over the code and like where you are headed with this.  I had a few ideas, let me know what you think.

1. What if get_source() was more like parse_uri in that it took "char **" as output parameters.  That way we can decouple setting the sip_pvt stringfields from the parsing function making it unit testable.  At that point it would be good to move get_source into reqresp_parser.c.

2. What if instead of calling the new get_source() function before the functions that require the "From" header information we just detected whether or not we needed to retrieve that information or not within the function it is required in.  I feel a little uneasy about decoupling the parsing from those functions since it would require us keeping track of making sure get_source() was called before them.  From a maintenance standpoint that might be a little rough. 

- David


On 2010-06-11 05:53:18, Nick Lewis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/701/
> -----------------------------------------------------------
> 
> (Updated 2010-06-11 05:53:18)
> 
> 
> 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
> 
>