[Home]

Summary:ASTERISK-04202: [patch] Voicemail Web Application Synchronization
Reporter:ssljivic (ssljivic)Labels:
Date Opened:2005-05-17 08:05:00Date Closed:2008-01-15 15:40:18.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) vm-web-sync-1.0.patch
( 1) vm-web-sync-1.1.1.patch
( 2) vm-web-sync-1.1.patch
Description:Hi,

I have added synchronization to the voicemail web application.

My disclaimer is on file.

Regards,
Stojan Sljivic
Comments:By: Russell Bryant (russell) 2005-05-18 21:22:29

I am definitely not a perl expert, so someone else needs to review this
By: Mark Monnin (wrmem) 2005-05-18 22:41:37

Any reason the patch is using "local" instead of "my" for variable declarations?
By: ssljivic (ssljivic) 2005-05-19 02:50:10

I have converted "local" to "my".

Regards
By: Mark Monnin (wrmem) 2005-05-19 12:45:28

Other suggestions (if you wish):

Declare subroutine variables before use (e.g. my $rfile = ...).  
Don't use "$rfile" as an argument when $rfile (no doublequotes) is sufficient).
And a useles nit: "move/delte" => "move/delete"


The untaint looks good (only matches A-Z, a-z, 1-9, 0, _(underscore), -(hyphen), @ (at-sign), and .(period)).  (I even had to double check it because you got me with the "." in the character class - wasn't sure that would be non-special - learnt somthing too!)


Also, just thinking out loud - lock_path (as well as the original ast_lock_path in pbx.c) doesn't handle stale locks at all.
By: ssljivic (ssljivic) 2005-05-20 02:33:06

Hi wrmem,

Thanks for the hints. I have applied them (new patch vm-web-sync-1.1.1.patch uploaded).

You are right regarding the stale locks.
Do you have any suggestion on how to handle them?

Regards,
Stojan Sljivic
By: Mark Spencer (markster) 2005-05-25 13:44:11

This still needs code review and functionality testing, please.
By: Michael Jerris (mikej) 2005-07-05 22:13:18

brian, can you code review the perl please.
By: Kevin P. Fleming (kpfleming) 2005-07-07 19:26:35

I've reviewed the code, it appears to be correct. Committed to CVS HEAD, thanks!
By: Digium Subversion (svnbot) 2008-01-15 15:40:18.000-0600

Repository: asterisk
Revision: 6055

U   trunk/contrib/scripts/vmail.cgi

------------------------------------------------------------------------
r6055 | kpfleming | 2008-01-15 15:40:17 -0600 (Tue, 15 Jan 2008) | 2 lines

add path-locking to voicemail CGI app (bug ASTERISK-4202)

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

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