[asterisk-dev] [Code Review] Tab completion on optional arguments can get discombobulated when there is a command containing a substring of another command given the proper order

[jrose]

> On 2011-04-14 17:28:02, Russell Bryant wrote:
> > /branches/1.6.2/main/cli.c, lines 1757-1761
> > <https://reviewboard.asterisk.org/r/1180/diff/4/?file=16167#file16167line1757>
> >
> >     How do you know pos isn't at the beginning of cli_word?  It shouldn't be if the input was valid ... do you _know_ that though?

Some experimentation lead me to see that if the user enters [<cmd> as their word, it'll back up during the comparison and see something it shouldn't see... in every case I tested an end of string character... which is why this is easy to overlook.

Strictly speaking, this is always going to happen, but to make things nice and simple, I'm adding the following condition to the if statement you have highlighted in here:
pos != cli_word &&

Uploading new diff with slightly revised comments.


- jrose


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


On 2011-04-14 15:42:43, jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1180/
> -----------------------------------------------------------
> 
> (Updated 2011-04-14 15:42:43)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> CLI tab completion can be confused by command options if there is some substring matching going on.  This has only been the case for commands once so far that I've actually been able to find, and it only existed in 1.6.2.
> 
> dahdi show channels has options [trunkgroup|group|context]
> 
> This example is tab completing against group.  If dahdi show channels group is in cli and the user tabs, the word group will be added again.  This can be repeated infinitely until the cli starts spitting argument errors.
> 
> 
> When the tab completion is being performed in ast_cli_generator, word_match is invoked using the cli word "group" against the command string [trunkgroup|group|context].  Inside this message, a string pointer pos is set using strcasestr(cli_word, cmd) which sets the cli command to the first occurence of the word in the command string.  Unfortunately this message is naive to the occurence of substrings, so the resulting pos for group is:
> 
> "group|group|context]"
> 
> instead of the expected "group|context]".
> 
> An if statement near the end of the word_match function evaluates the string to see if the word is a command word by checking if the character before it is a a cli reserved character (such as {, |, or [)
> 
> since the character before pos in this case is 'k', it skips this if clause and returns -1 instead of the expected 1, causing it to match back in the ast_cli_generator function.
> 
> 
> My fix for this is to search against the various reserved characters concatenated with the cli word.  Once one is found, another search is done on that substring to put it at the actual command so it is looking at the actual word rather than <reserved_character>word.  I think there is probably a way to search against all of these characters at once without having to rely on string concatenation, possibly with regular expressions, but I'm not especially certain on that and if anyone has any recommendations on that I'd be glad to follow them.
> 
> 
> This addresses bug 17494.
>     https://issues.asterisk.org/view.php?id=17494
> 
> 
> Diffs
> -----
> 
>   /branches/1.6.2/main/cli.c 313276 
> 
> Diff: https://reviewboard.asterisk.org/r/1180/diff
> 
> 
> Testing
> -------
> 
> Obviously a lot of tab completion on various inputs of cli words was used, but I also was using some fairly verbose checks of what was going on in the word_match function to make sure things were happening the way I think they were happening.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110415/77782270/attachment.htm>