[asterisk-dev] [Code Review] SIP: decode uri after parsing

[Nick Lewis]

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


I may be wrong but I have a feeling that the exact nature of the uri decode (for example which characters are delimiters and which are field content) sometimes depends on the type of header being decoded so this uri-type information may need to be supplied to ast_uri_decode

- Nick


On 2009-12-21 09:35:32, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/450/
> -----------------------------------------------------------
> 
> (Updated 2009-12-21 09:35:32)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> In chan_sip.c ast_uri_decode is called on the entire URI instead of it's individual parts after it is parsed.  This is not good as ast_uri_decode can introduce special characters back into the URI which can mess up parsing.  This patch resolves this by not decoding a URI until parsing is completely done.  There are many instances where we check to see if pedantic checking is enabled before we decode a URI.  In these cases a new macro, SIP_PEDANTIC_DECODE, is used on the individual parsed segments of the URI rather than constantly putting if (pedantic) { decode() } checks everywhere in the code.  In the areas where ast_uri_decode is not dependent upon pedantic checking this macro is not used, but decoding is still moved to each individual part of the URI.  The only behavior that should change from this patch is the time at which decoding occurs.
> 
> Since I had to look over every place URI parsing occurs to create this patch, I found several places where we use duplicate code for parsing.  To consolidate the code, those areas have updated to use the parse_uri() function where possible.
> 
> 
> This addresses bug 16299.
>     https://issues.asterisk.org/view.php?id=16299
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 235738 
> 
> Diff: https://reviewboard.asterisk.org/r/450/diff
> 
> 
> Testing
> -------
> 
> Verified basic URI parsing is still working by placing calls and completing transfers.
> 
> 
> Thanks,
> 
> David
> 
>