[asterisk-dev] [Code Review] ast_uri_encode() behavior change

[David Vossel]

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


Note, the test code currently within chan_sip will be moved into it's own utils_test.c file.  The code will remain the same, but the location will change.

- David


On 2009-12-23 15:28:15, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/451/
> -----------------------------------------------------------
> 
> (Updated 2009-12-23 15:28:15)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch changes ast_uri_encode()'s behavior when doreserved is enabled.  Previously when doreserved was enabled only a small set of reserved characters were encoded.  This set was comprised primarily of the reserved characters defined in RFC3261 section 25.1, but contained other characters as well.  Rather than only escaping the reserved set, doreserved now escapes all characters not within the unreserved set as defined by RFC 3261 and RFC 2396.  Also, the 'doreserved' variable has been renamed to 'do_special_char' in attempts to avoid confusion.
> 
> When doreserve is not enabled, the previous logic of only encoding the characters <= 0X1F and > 0X7f remains.
> 
> In RFC 3261 and RFC 2396 the unreserved character set is defined by all alphanumeric characters and a small number of characters defined in the mark set.
> mark        =  "-" / "_" / "." / "!" / "~" / "*" / "'"
>                      / "(" / ")"
> unreserved  =  alphanum / mark
> 
> 
> I have viewed several discussions involving this code lately.  My hopes are that this patch will prompt continued discussion and help bring us to a solution we can all agree upon. 
> 
> This patch contains logic written by both wdoekes and myself.  Discussions pertaining to this issue may either be discussed here or directed to (issue #16299).
> 
> 
> This addresses bug 16299.
>     https://issues.asterisk.org/view.php?id=16299
> 
> 
> Diffs
> -----
> 
>   /trunk/main/utils.c 236309 
>   /trunk/include/asterisk/utils.h 236309 
>   /trunk/channels/chan_sip.c 236309 
> 
> Diff: https://reviewboard.asterisk.org/r/451/diff
> 
> 
> Testing
> -------
> 
> I wrote a Unit Test to verify ast_uri_encode's output.  The results are below.
> 
> START  main/utils/ - uri_encode_test 
> 
> Input before executing ast_uri_encode:
> abcdefghijklmnopurstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 ~`!@#$%^&*()_-+={[}]|\:;"'<,>.?/
> Output expected for ast_uri_encode with enabling do_special_char:
> abcdefghijklmnopurstuvwxyz%20ABCDEFGHIJKLMNOPQRSTUVWXYZ%201234567890%20~%60!%40%23%24%25%5e%26*()_-%2b%3d%7b%5b%7d%5d%7c%5c%3a%3b%22'%3c%2c%3e.%3f%2f
> 
> Output after enabling do_special_char:
> abcdefghijklmnopurstuvwxyz%20ABCDEFGHIJKLMNOPQRSTUVWXYZ%201234567890%20~%60!%40%23%24%25%5e%26*()_-%2b%3d%7b%5b%7d%5d%7c%5c%3a%3b%22'%3c%2c%3e.%3f%2f
> Decoded string matched original input
> 
> Output after disabling do_special_char:
> abcdefghijklmnopurstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 ~`!@#$%^&*()_-+={[}]|\:;"'<,>.?/
> Decoded string matched original input
> 
> END    main/utils/ - uri_encode_test Time: 0ms Result: PASS 
> 
> 
> Thanks,
> 
> David
> 
>