[Home]

Summary:ASTERISK-18308: Problem with batch-creation of astdb entries
Reporter:Tzafrir Cohen (tzafrir)Labels:
Date Opened:2011-08-22 01:12:04Date Closed:2012-04-19 14:45:46
Priority:MinorRegression?
Status:Closed/CompleteComponents:Core/General
Versions:1.8.5.0 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) astcmd
( 1) fix_multipe_cli_commands.diff
( 2) issueA18308_buffer_consecutive_ctl_commands.patch
Description:(I'm not completely sure that the following is a legitimate bug, but anyway)

Following the Freepbxdb.pm modules I added to dahdi-perl ( http://svnview.digium.com/svn/dahdi?view=revision&revision=10142 ), and some potential astdb corruption cases I encountered to make a simpler test case of a direct write of commands to the asterisk.ctl .

The attached astcmd is a script that uses the same code and accepts any commands from the standard input.

$ (echo database deltree tzafrir; for i in `seq 50`; do echo database put tzafrir test$i 123; done) | astcmd
$ asterisk -rx 'database show tzafrir'  | grep ^/  | wc -l
47

Whereas:

$ asterisk -rnx 'database deltree tzafrir'>/dev/null; for i in `seq 50`; do asterisk -rnx "database put tzafrir test$i 1234" >/dev/null; done
$ asterisk -rx 'database show tzafrir'  | grep ^/  | wc -l
50

That is, using astcmd, 3 are missing. It is always the same 3: entries no. 16, 32 and 48 .
Comments:By: Matt Jordan (mjordan) 2011-12-27 08:44:25.715-0600

Can you attach the astcmd script?

By: Tzafrir Cohen (tzafrir) 2012-02-09 17:50:59.750-0600

Oops. Now with script attached.

By: Walter Doekes (wdoekes) 2012-02-10 15:48:40.660-0600

It's the char tmp[512]; in main/asterisk.c netconsole(). The 16th, 32th and 48th entry fall on multiples of that and get broken to bits.

By: Walter Doekes (wdoekes) 2012-02-13 15:04:44.637-0600

Attached, a bug-prone, yet working solution ;)

A cleaner way could be to enlarge the buffer until res is finally less than sizeof(cmdbuf). Pros: less bug-prone. Cons:  we'd have to put an arbitrary limit on that to avoid malicious memory consumption, it doesn't start processing until the complete buffer is read, there's a hardcoded limit of 512 bytes per command in ast_cli_command_multiple_full() so reading more bytes at once isn't useful.

By: Terry Wilson (twilson) 2012-04-18 21:48:43.625-0500

I attached a similar patch to walter's, fix_multiple_cli_commands.diff, that I think is a little easier to read and might handle some more edge cases. In particular I was worried about the counting backward looking for NULLs, since I think old NULLs from previous reads could have ended up getting picked up.

This worked for my tests, anyway. You might check and see if it works for you as well or if you see any problems with it.

By: Walter Doekes (wdoekes) 2012-04-19 02:16:52.468-0500

Testing by eye only.

+1 on readability.
(-1 on the edges cases until you prove it :P)

Perhaps a small comment above the call to ast_cli_command_multiple_full that it will only execute those commands terminated by a null. And make end_buf a char *const, to clarify that it's immutable.

(Leaving in feedback so tzafrir can test.)