[Home]

Summary:ASTERISK-13817: [patch] Voicemail(ARGS) is limtted to 1024 characters, large 'blast' groups are silently left off
Reporter:Philippe Lindheimer (p_lindheimer)Labels:
Date Opened:2009-03-24 16:53:32Date Closed:2009-05-11 18:36:06
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20090413__bug14739.diff.txt
( 1) 20090417__bug14739.diff.txt
Description:When sending voicemail to several users in the supported format:

Voicemail(200@default&201@default& ... 299@default,s)

the app_voicemail truncates the resulting parameter to 1024 characters. In FreePBX, there is a blast group that accumulates these into a channel variable. Therefore, the size that should be allowed should be at least equivalent to the max size of a channel variable. (Or larger if a command line can be longer). Unless of course there are other limitations...

In FreePBX, this is called as:

exten => 1,1,VoiceMail(${GRPLIST:1},s)                                                                                                            

where the list was previously accumulated in GRPLIST.

The following change reportedly fixes the issue until the new limitation is met:

--- app_voicemail.c     (revision 184036)
+++ app_voicemail.c     (working copy)
@@ -4962,7 +4962,7 @@
       char fmt[80];
       char *context;
       char ecodes[17] = "#";
-       char tmp[1024] = "";
+       char tmp[2048] = "";
       char *tmpptr;
       struct ast_vm_user *vmu;
       struct ast_vm_user svm;

So it would seem that either a much larger tmp variable should be created to address very large lists, or it should be dynamically allocated based on the sizeof the ext argument that is passed to it.

(note - I marked it 'major' because the dialplan is silently corrupting the data, seemed like it was more than minor, less than major)...

****** STEPS TO REPRODUCE ******

create a very large voicemail blast group that exceeds 1024 characters in the passed argument.

****** ADDITIONAL INFORMATION ******

This appears to exist in Trunk, 1.6 and 1.4 versions (and probably before)

See the forum discussion on the issue here:

http://freepbx.org/forum/freepbx/users/vm-blast-group-limitation-on-number-of-extensions-in-group



Update (Leif, 2009/04/13):  Branch is available here --> http://svn.digium.com/svn/asterisk/team/tilghman/str_substitution/
Comments:By: Leif Madsen (lmadsen) 2009-04-13 13:00:10

I have talked to Tilghman on IRC, and this is basically a "won't fix" in any currently released branches as 1024 is typically "good enough for most people". However, in the upcoming 1.6.3 (pending) this will be resolved to use the changes Tilghman will be be making in his str_substitution branch (http://svn.digium.com/svn/asterisk/team/tilghman/str_substitution/) that will fix it permanently going forward.

Thanks!
Leif.
By: Tilghman Lesher (tilghman) 2009-04-13 18:33:51

Or I could do something like this, which will work on 1.6.0 and greater.
By: Philippe Lindheimer (p_lindheimer) 2009-04-17 12:01:30

If 1024 is "good enough for most people" that would imply that Asterisk is limited to roughly 75 extensions for an installation that wants the ability to do voicemail blast groups to all employees. This doesn't do Asterisk justice as we all know it is MUCH more capable then that. The 1024 is arbitrary and my understanding is that the suggested patch is not applicable to the 1.4 branch of Asterisk.

I can understand if there is not an 'elegant' solution for 1.4 as proposed in the supplied patch. However, bumping this up to a bigger number such as 4096 (or maybe double that) will take this from effecting maybe "1%" of installations to maybe "0.01%" as it would mask the issue until someone tried to do over 300 simultaneous extensions.

I would also suggest that as long as the value stays static, it's length is tested against the static length (currently 1024) and an error is logged to the log file. As it stands, it results in a silent failure which is never a good thing...
By: Tilghman Lesher (tilghman) 2009-04-17 13:39:29

Yes, but you also have to understand that extending the amount of stack space that we use COULD affect existing installations, because using more stack means that Voicemail called from several Macros deep may no longer work, due
to lack of available space.  Keep in mind that this is not technically a bug, just a limitation, and the request to expand it is therefore a feature request.  The 1.4 series is feature-frozen, therefore any such expansion is a violation of that policy.

"I think it should work" is a nice wish, but on the balance, I have to reject this expansion as being too great a risk for existing installations.
By: Tilghman Lesher (tilghman) 2009-04-17 13:54:17

I took a look at the stack usage, and I found 300 bytes that we can shift to this temporary buffer, plus the warning.  I think that's the best I can offer.
By: Philippe Lindheimer (p_lindheimer) 2009-04-17 16:02:10

thanks, that clarifies the other issues involved that I was not aware of. Any more that can be provided, even if only 300 bytes, will help:-)
By: Leif Madsen (lmadsen) 2009-04-21 22:49:41

Have you had a chance to try this out?
By: Philippe Lindheimer (p_lindheimer) 2009-04-22 17:08:40

not yet - I'm hoping to try this and another bug fix tomorrow (been teaching a class all week before that).
By: Philippe Lindheimer (p_lindheimer) 2009-04-22 20:10:54

got a chance to test. Using 8 digit extensions with the "default" vm context, it was able to handle 77 extensions in a blast group.

The only question, shouldn't this be an ERROR in the log file vs. a WARNING? Otherwise, looks good.
By: Tilghman Lesher (tilghman) 2009-04-23 11:03:31

No, the established difference between a WARNING and an ERROR is that an ERROR causes the activity to be terminated.  A WARNING means that an exceptional condition occurred, but the application can proceed anyway.
By: Leif Madsen (lmadsen) 2009-05-11 15:51:43

Since it looks like the only remaining issue here was a discussion about why we were using WARNING instead of ERROR, and that everything else was good (functionality wise) I'm going to set this Ready for Review.
By: Digium Subversion (svnbot) 2009-05-11 17:48:24

Repository: asterisk
Revision: 193755

U   branches/1.4/apps/app_voicemail.c

------------------------------------------------------------------------
r193755 | tilghman | 2009-05-11 17:48:23 -0500 (Mon, 11 May 2009) | 18 lines

Move 300 bytes around on the stack, to make more room for an extension buffer.
This allows more concurrent extensions to be copied for a single voicemail,
without creating a possibility of upsetting existing users, where a dialplan
could run out of stack space where it had run fine before.  Alternatively,
we could have allocated off the heap, but that is a larger change and would
have increased the chance for instability introduced by this change.

This is really solved starting in 1.6.0.11, as the use of an ast_str buffer
allows an unlimited number of extensions (up to available memory).  We
additionally create a new warning message when the buffer length is exceeded,
permitting administrators to see an issue after the fact, whereas previously
the list was silently truncated.
(closes issue ASTERISK-13817)
Reported by: p_lindheimer
Patches:
      20090417__bug14739.diff.txt uploaded by tilghman (license 14)
Tested by: p_lindheimer

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=193755
By: Digium Subversion (svnbot) 2009-05-11 17:50:51

Repository: asterisk
Revision: 193756

_U  trunk/

------------------------------------------------------------------------
r193756 | tilghman | 2009-05-11 17:50:51 -0500 (Mon, 11 May 2009) | 25 lines

Recorded merge of revisions 193755 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r193755 | tilghman | 2009-05-11 17:48:20 -0500 (Mon, 11 May 2009) | 18 lines
 
 Move 300 bytes around on the stack, to make more room for an extension buffer.
 This allows more concurrent extensions to be copied for a single voicemail,
 without creating a possibility of upsetting existing users, where a dialplan
 could run out of stack space where it had run fine before.  Alternatively,
 we could have allocated off the heap, but that is a larger change and would
 have increased the chance for instability introduced by this change.
 
 This is really solved starting in 1.6.0.11, as the use of an ast_str buffer
 allows an unlimited number of extensions (up to available memory).  We
 additionally create a new warning message when the buffer length is exceeded,
 permitting administrators to see an issue after the fact, whereas previously
 the list was silently truncated.
 (closes issue ASTERISK-13817)
  Reported by: p_lindheimer
  Patches:
        20090417__bug14739.diff.txt uploaded by tilghman (license 14)
  Tested by: p_lindheimer
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=193756
By: Digium Subversion (svnbot) 2009-05-11 18:23:19

Repository: asterisk
Revision: 193781

_U  branches/1.6.0/
U   branches/1.6.0/apps/app_voicemail.c

------------------------------------------------------------------------
r193781 | tilghman | 2009-05-11 18:23:19 -0500 (Mon, 11 May 2009) | 32 lines

Recorded merge of revisions 193756 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r193756 | tilghman | 2009-05-11 17:50:47 -0500 (Mon, 11 May 2009) | 25 lines
 
 Recorded merge of revisions 193755 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r193755 | tilghman | 2009-05-11 17:48:20 -0500 (Mon, 11 May 2009) | 18 lines
   
   Move 300 bytes around on the stack, to make more room for an extension buffer.
   This allows more concurrent extensions to be copied for a single voicemail,
   without creating a possibility of upsetting existing users, where a dialplan
   could run out of stack space where it had run fine before.  Alternatively,
   we could have allocated off the heap, but that is a larger change and would
   have increased the chance for instability introduced by this change.
   
   This is really solved starting in 1.6.0.11, as the use of an ast_str buffer
   allows an unlimited number of extensions (up to available memory).  We
   additionally create a new warning message when the buffer length is exceeded,
   permitting administrators to see an issue after the fact, whereas previously
   the list was silently truncated.
   (closes issue ASTERISK-13817)
    Reported by: p_lindheimer
    Patches:
          20090417__bug14739.diff.txt uploaded by tilghman (license 14)
    Tested by: p_lindheimer
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=193781
By: Digium Subversion (svnbot) 2009-05-11 18:35:56

Repository: asterisk
Revision: 193822

_U  branches/1.6.1/
U   branches/1.6.1/apps/app_voicemail.c

------------------------------------------------------------------------
r193822 | tilghman | 2009-05-11 18:35:56 -0500 (Mon, 11 May 2009) | 32 lines

Recorded merge of revisions 193756 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r193756 | tilghman | 2009-05-11 17:50:47 -0500 (Mon, 11 May 2009) | 25 lines
 
 Recorded merge of revisions 193755 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r193755 | tilghman | 2009-05-11 17:48:20 -0500 (Mon, 11 May 2009) | 18 lines
   
   Move 300 bytes around on the stack, to make more room for an extension buffer.
   This allows more concurrent extensions to be copied for a single voicemail,
   without creating a possibility of upsetting existing users, where a dialplan
   could run out of stack space where it had run fine before.  Alternatively,
   we could have allocated off the heap, but that is a larger change and would
   have increased the chance for instability introduced by this change.
   
   This is really solved starting in 1.6.0.11, as the use of an ast_str buffer
   allows an unlimited number of extensions (up to available memory).  We
   additionally create a new warning message when the buffer length is exceeded,
   permitting administrators to see an issue after the fact, whereas previously
   the list was silently truncated.
   (closes issue ASTERISK-13817)
    Reported by: p_lindheimer
    Patches:
          20090417__bug14739.diff.txt uploaded by tilghman (license 14)
    Tested by: p_lindheimer
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=193822
By: Digium Subversion (svnbot) 2009-05-11 18:36:05

Repository: asterisk
Revision: 193823

_U  branches/1.6.2/
U   branches/1.6.2/apps/app_voicemail.c

------------------------------------------------------------------------
r193823 | tilghman | 2009-05-11 18:36:05 -0500 (Mon, 11 May 2009) | 32 lines

Recorded merge of revisions 193756 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r193756 | tilghman | 2009-05-11 17:50:47 -0500 (Mon, 11 May 2009) | 25 lines
 
 Recorded merge of revisions 193755 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r193755 | tilghman | 2009-05-11 17:48:20 -0500 (Mon, 11 May 2009) | 18 lines
   
   Move 300 bytes around on the stack, to make more room for an extension buffer.
   This allows more concurrent extensions to be copied for a single voicemail,
   without creating a possibility of upsetting existing users, where a dialplan
   could run out of stack space where it had run fine before.  Alternatively,
   we could have allocated off the heap, but that is a larger change and would
   have increased the chance for instability introduced by this change.
   
   This is really solved starting in 1.6.0.11, as the use of an ast_str buffer
   allows an unlimited number of extensions (up to available memory).  We
   additionally create a new warning message when the buffer length is exceeded,
   permitting administrators to see an issue after the fact, whereas previously
   the list was silently truncated.
   (closes issue ASTERISK-13817)
    Reported by: p_lindheimer
    Patches:
          20090417__bug14739.diff.txt uploaded by tilghman (license 14)
    Tested by: p_lindheimer
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=193823