[asterisk-dev] [Code Review] 3923: CallerID: Fix parsing of malformed callerid strings

[rmudgett]

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



branches/1.8/main/callerid.c
<https://reviewboard.asterisk.org/r/3923/#comment23574>

    This feature of quote_count is effectively dead code as you never pass -1.



branches/1.8/main/callerid.c
<https://reviewboard.asterisk.org/r/3923/#comment23575>

    I am wondering if ast_callerid_parse() should be escaping the quotes at all.  The job of ast_callerid_parse() is to take the string and separate it into a name and number string.  Escaping characters seems wrong.  If anything, the routine should be unescaping character sequences.  The consumers likely don't know how to handle escaped quotes or may need to escape characters differently.



branches/1.8/tests/test_callerid.c
<https://reviewboard.asterisk.org/r/3923/#comment23576>

    red



branches/1.8/tests/test_callerid.c
<https://reviewboard.asterisk.org/r/3923/#comment23577>

    red



branches/1.8/tests/test_callerid.c
<https://reviewboard.asterisk.org/r/3923/#comment23573>

    Rather than a bunch of redundant tests checking minor variations in the string.  How about one test that works through an array of input string variatons and the expected broken out strings:
    struct expect {
      char *orig;
      char *name;
      char *number;
    };
    struct expect array[] = {
      { "name <number>", "name", "number" },
      { "\"name\" <number>", "name", "number" },
      { "name", "name", NULL },
      { "1234", NULL, "1234" }
    }
    
    The last two entries above are also valid possibilities for the input string to ast_callerid_parse().


- rmudgett


On Aug. 18, 2014, 8:55 p.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3923/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2014, 8:55 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This allows the callerid parsing function to handle malformed input strings and strings containing escaped and unescaped double quotes. This also adds a unittest to cover many of the cases where the parsing algorithm previously failed.
> 
> 
> Diffs
> -----
> 
>   branches/1.8/tests/test_callerid.c PRE-CREATION 
>   branches/1.8/res/res_agi.c 421326 
>   branches/1.8/main/privacy.c 421326 
>   branches/1.8/main/manager.c 421326 
>   branches/1.8/main/callerid.c 421326 
>   branches/1.8/include/asterisk/callerid.h 421326 
>   branches/1.8/channels/chan_unistim.c 421326 
>   branches/1.8/channels/chan_misdn.c 421326 
>   branches/1.8/apps/app_voicemail.c 421326 
>   branches/1.8/apps/app_dial.c 421326 
> 
> Diff: https://reviewboard.asterisk.org/r/3923/diff/
> 
> 
> Testing
> -------
> 
> Ran the unittest.
> 
> 
> Thanks,
> 
> opticron
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140819/88e479f5/attachment-0001.html>