[svn-commits] mmichelson: branch mmichelson/trunk-digiumphones r361609 - in /team/mmichelso...
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Fri Apr 6 17:05:11 CDT 2012
Author: mmichelson
Date: Fri Apr 6 17:05:07 2012
New Revision: 361609
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=361609
Log:
Many many bug fixes to new voicemail APIs.
* Fixes many cases where NULL inputs could cause crashes or
falsely return successful even though the operation failed.
* Fixes cases where an invalid number of messages was passed to
the new functions.
* Adds range and existence checking for message indexes to help
improve atomicity of operations.
* Changes ast_vm_msg_play() to set the "heard" field for a message
until after the message has been played back.
* Remove the destination folder option from ast_vm_msg_forward()
(issue AST-845)
(issue AST-853)
(issue AST-855)
(issue AST-851)
(issue AST-849)
(issue AST-857)
(issue AST-859)
(issue AST-861)
(issue AST-863)
(issue AST-865)
Modified:
team/mmichelson/trunk-digiumphones/apps/app_voicemail.c
team/mmichelson/trunk-digiumphones/include/asterisk/app_voicemail.h
Modified: team/mmichelson/trunk-digiumphones/apps/app_voicemail.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/trunk-digiumphones/apps/app_voicemail.c?view=diff&rev=361609&r1=361608&r2=361609
==============================================================================
--- team/mmichelson/trunk-digiumphones/apps/app_voicemail.c (original)
+++ team/mmichelson/trunk-digiumphones/apps/app_voicemail.c Fri Apr 6 17:05:07 2012
@@ -14595,6 +14595,11 @@
int inbox_index = 0;
int old_index = 1;
+ if (ast_strlen_zero(mailbox)) {
+ ast_log(LOG_WARNING, "Cannot create a mailbox snapshot since no mailbox was specified\n");
+ return NULL;
+ }
+
memset(&vmus, 0, sizeof(vmus));
if (!(ast_strlen_zero(folder))) {
@@ -14728,6 +14733,51 @@
return str;
}
+/*!
+ * \brief common bounds checking and existence check for Voicemail API functions.
+ *
+ * \details
+ * This is called by ast_vm_msg_move, ast_vm_msg_remove, and ast_vm_msg_forward to
+ * ensure that data passed in are valid. This tests the following:
+ *
+ * 1. No negative indexes are given.
+ * 2. No index greater than the highest message index for the folder is given.
+ * 3. All message indexes given point to messages that exist.
+ *
+ * \param vms The voicemail state corresponding to an open mailbox
+ * \param msg_ids An array of message identifiers
+ * \param num_msgs The number of identifiers in msg_ids
+ *
+ * \retval -1 Failure
+ * \retval 0 Success
+ */
+static int message_range_and_existence_check(struct vm_state *vms, int *msg_ids, size_t num_msgs)
+{
+ int i;
+ int res = 0;
+ for (i = 0; i < num_msgs; ++i) {
+ int cur_msg = msg_ids[i];
+ if (cur_msg < 0) {
+ ast_log(LOG_WARNING, "Message has negative index\n");
+ res = -1;
+ break;
+ }
+ if (vms->lastmsg < cur_msg) {
+ ast_log(LOG_WARNING, "Message %d is out of range. Last message is %d\n", cur_msg, vms->lastmsg);
+ res = -1;
+ break;
+ }
+ make_file(vms->fn, sizeof(vms->fn), vms->curdir, cur_msg);
+ if (!EXISTS(vms->curdir, cur_msg, vms->fn, NULL)) {
+ ast_log(LOG_WARNING, "Message %d does not exist.\n", cur_msg);
+ res = -1;
+ break;
+ }
+ }
+
+ return res;
+}
+
static void notify_new_state(struct ast_vm_user *vmu)
{
int new = 0, old = 0, urgent = 0;
@@ -14744,7 +14794,6 @@
const char *from_folder,
const char *to_mailbox,
const char *to_context,
- const char *to_folder,
size_t num_msgs,
int *msg_ids,
int delete_old)
@@ -14755,17 +14804,32 @@
struct ast_config *msg_cfg;
struct ast_flags config_flags = { CONFIG_FLAG_NOCACHE };
char filename[PATH_MAX];
- int from_folder_index = get_folder_by_name(from_folder);
- int to_folder_index = get_folder_by_name(to_folder);
+ int from_folder_index;
int open = 0;
int res = 0;
int i;
+ if (ast_strlen_zero(from_mailbox) || ast_strlen_zero(to_mailbox)) {
+ ast_log(LOG_WARNING, "Cannot forward message because either the from or to mailbox was not specified\n");
+ return -1;
+ }
+
+ if (!num_msgs) {
+ ast_log(LOG_WARNING, "Invalid number of messages specified to forward: %zu\n", num_msgs);
+ return -1;
+ }
+
+ if (ast_strlen_zero(from_folder)) {
+ ast_log(LOG_WARNING, "Cannot forward message because the fromfolder was not specified\n");
+ return -1;
+ }
+
memset(&vmus, 0, sizeof(vmus));
memset(&to_vmus, 0, sizeof(to_vmus));
memset(&from_vms, 0, sizeof(from_vms));
- if (to_folder_index == -1 || from_folder_index == -1) {
+ from_folder_index = get_folder_by_name(from_folder);
+ if (from_folder_index == -1) {
return -1;
}
@@ -14792,20 +14856,30 @@
open = 1;
+ if ((from_vms.lastmsg + 1) < num_msgs) {
+ ast_log(LOG_WARNING, "Folder %s has less than %zu messages\n", from_folder, num_msgs);
+ res = -1;
+ goto vm_forward_cleanup;
+ }
+
+ if ((res = message_range_and_existence_check(&from_vms, msg_ids, num_msgs) < 0)) {
+ goto vm_forward_cleanup;
+ }
+
+ /* Now we actually forward the messages */
for (i = 0; i < num_msgs; i++) {
int cur_msg = msg_ids[i];
int duration = 0;
const char *value;
- if (cur_msg >= 0 && from_vms.lastmsg < cur_msg) {
- /* msg does not exist */
- ast_log(LOG_WARNING, "msg %d does not exist to forward. Last msg is %d\n", cur_msg, from_vms.lastmsg);
- continue;
- }
make_file(from_vms.fn, sizeof(from_vms.fn), from_vms.curdir, cur_msg);
snprintf(filename, sizeof(filename), "%s.txt", from_vms.fn);
RETRIEVE(from_vms.curdir, cur_msg, vmu->mailbox, vmu->context);
msg_cfg = ast_config_load(filename, config_flags);
+ /* XXX This likely will not fail since we previously ensured that the
+ * message we are looking for exists. However, there still could be some
+ * circumstance where this fails, so atomicity is not guaranteed.
+ */
if (!msg_cfg || msg_cfg == CONFIG_STATUS_FILEINVALID) {
DISPOSE(from_vms.curdir, cur_msg);
continue;
@@ -14857,11 +14931,29 @@
{
struct vm_state vms;
struct ast_vm_user *vmu = NULL, vmus;
- int old_folder_index = get_folder_by_name(oldfolder);
- int new_folder_index = get_folder_by_name(newfolder);
+ int old_folder_index;
+ int new_folder_index;
int open = 0;
int res = 0;
int i;
+
+ if (ast_strlen_zero(mailbox)) {
+ ast_log(LOG_WARNING, "Cannot move message because no mailbox was specified\n");
+ return -1;
+ }
+
+ if (!num_msgs) {
+ ast_log(LOG_WARNING, "Invalid number of messages specified to move: %zu\n", num_msgs);
+ return -1;
+ }
+
+ if (ast_strlen_zero(oldfolder) || ast_strlen_zero(newfolder)) {
+ ast_log(LOG_WARNING, "Cannot move message because either oldfolder or newfolder was not specified\n");
+ return -1;
+ }
+
+ old_folder_index = get_folder_by_name(oldfolder);
+ new_folder_index = get_folder_by_name(newfolder);
memset(&vmus, 0, sizeof(vmus));
memset(&vms, 0, sizeof(vms));
@@ -14887,12 +14979,12 @@
open = 1;
- for (i = 0; i < num_msgs; i++) {
- if (vms.lastmsg < old_msg_nums[i]) {
- /* msg does not exist */
- res = -1;
- goto vm_move_cleanup;
- }
+ if ((res = message_range_and_existence_check(&vms, old_msg_nums, num_msgs)) < 0) {
+ goto vm_move_cleanup;
+ }
+
+ /* Now actually move the message */
+ for (i = 0; i < num_msgs; ++i) {
if (save_to_folder(vmu, &vms, old_msg_nums[i], new_folder_index, (new_msg_nums + i))) {
res = -1;
goto vm_move_cleanup;
@@ -14937,6 +15029,16 @@
int res = 0;
int i;
+ if (ast_strlen_zero(mailbox)) {
+ ast_log(LOG_WARNING, "Cannot remove message because no mailbox was specified\n");
+ return -1;
+ }
+
+ if (!num_msgs) {
+ ast_log(LOG_WARNING, "Invalid number of messages specified to remove: %zu\n", num_msgs);
+ return -1;
+ }
+
memset(&vmus, 0, sizeof(vmus));
memset(&vms, 0, sizeof(vms));
@@ -14963,13 +15065,17 @@
open = 1;
+ if ((vms.lastmsg + 1) < num_msgs) {
+ ast_log(LOG_WARNING, "Folder %s has less than %zu messages\n", folder, num_msgs);
+ res = -1;
+ goto vm_remove_cleanup;
+ }
+
+ if ((res = message_range_and_existence_check(&vms, msgs, num_msgs)) < 0) {
+ goto vm_remove_cleanup;
+ }
+
for (i = 0; i < num_msgs; i++) {
- if (vms.lastmsg < msgs[i]) {
- /* msg does not exist */
- ast_log(AST_LOG_ERROR, "Could not remove msg %d from folder %s because it does not exist.\n", msgs[i], folder);
- res = -1;
- goto vm_remove_cleanup;
- }
vms.deleted[msgs[i]] = 1;
}
@@ -15018,6 +15124,25 @@
int res = 0;
int open = 0;
int i;
+ char filename[PATH_MAX];
+ struct ast_config *msg_cfg;
+ struct ast_flags config_flags = { CONFIG_FLAG_NOCACHE };
+ int duration = 0;
+ const char *value;
+
+ if (ast_strlen_zero(mailbox)) {
+ ast_log(LOG_WARNING, "Cannot play message because no mailbox was specified\n");
+ return -1;
+ }
+
+ if (ast_strlen_zero(folder)) {
+ ast_log(LOG_WARNING, "Cannot play message because no folder was specified\n");
+ return -1;
+ }
+
+ if (ast_strlen_zero(msg_num)) {
+ ast_log(LOG_WARNING, "Cannot play message because no message number was specified\n");
+ }
memset(&vmus, 0, sizeof(vmus));
memset(&vms, 0, sizeof(vms));
@@ -15027,69 +15152,60 @@
}
if (!(vmu = find_user(&vmus, context, mailbox))) {
+ return -1;
+ }
+
+ i = get_folder_by_name(folder);
+ ast_copy_string(vms.username, mailbox, sizeof(vms.username));
+ vms.lastmsg = -1;
+ if ((res = open_mailbox(&vms, vmu, i)) < 0) {
+ ast_log(LOG_WARNING, "Could not open mailbox %s\n", mailbox);
goto play2_msg_cleanup;
}
-
- if (!ast_strlen_zero(msg_num) && !ast_strlen_zero(folder)) {
- char filename[PATH_MAX];
- struct ast_config *msg_cfg;
- struct ast_flags config_flags = { CONFIG_FLAG_NOCACHE };
- int duration = 0;
- const char *value;
-
- i = get_folder_by_name(folder);
- ast_copy_string(vms.username, mailbox, sizeof(vms.username));
- vms.lastmsg = -1;
- if ((res = open_mailbox(&vms, vmu, i)) < 0) {
- ast_log(LOG_WARNING, "Could not open mailbox %s\n", mailbox);
- res = -1;
- goto play2_msg_cleanup;
- }
- open = 1;
-
- vms.curmsg = atoi(msg_num);
- if (vms.curmsg > vms.lastmsg) {
- res = -1;
- goto play2_msg_cleanup;
- }
-
- /* Find the msg */
- make_file(vms.fn, sizeof(vms.fn), vms.curdir, vms.curmsg);
- snprintf(filename, sizeof(filename), "%s.txt", vms.fn);
- RETRIEVE(vms.curdir, vms.curmsg, vmu->mailbox, vmu->context);
-
- msg_cfg = ast_config_load(filename, config_flags);
- if (!msg_cfg || msg_cfg == CONFIG_STATUS_FILEINVALID) {
- DISPOSE(vms.curdir, vms.curmsg);
- res = -1;
- goto play2_msg_cleanup;
- }
- if ((value = ast_variable_retrieve(msg_cfg, "message", "duration"))) {
- duration = atoi(value);
- }
- ast_config_destroy(msg_cfg);
-
- vms.heard[vms.curmsg] = 1;
+ open = 1;
+
+ vms.curmsg = atoi(msg_num);
+ if (vms.curmsg > vms.lastmsg || vms.curmsg < 0) {
+ res = -1;
+ goto play2_msg_cleanup;
+ }
+
+ /* Find the msg */
+ make_file(vms.fn, sizeof(vms.fn), vms.curdir, vms.curmsg);
+ snprintf(filename, sizeof(filename), "%s.txt", vms.fn);
+ RETRIEVE(vms.curdir, vms.curmsg, vmu->mailbox, vmu->context);
+
+ msg_cfg = ast_config_load(filename, config_flags);
+ if (!msg_cfg || msg_cfg == CONFIG_STATUS_FILEINVALID) {
+ DISPOSE(vms.curdir, vms.curmsg);
+ res = -1;
+ goto play2_msg_cleanup;
+ }
+ if ((value = ast_variable_retrieve(msg_cfg, "message", "duration"))) {
+ duration = atoi(value);
+ }
+ ast_config_destroy(msg_cfg);
#ifdef IMAP_STORAGE
- /*IMAP storage stores any prepended message from a forward
- * as a separate file from the rest of the message
- */
- if (!ast_strlen_zero(vms.introfn) && ast_fileexists(vms.introfn, NULL, NULL) > 0) {
- wait_file(chan, &vms, vms.introfn);
- }
+ /*IMAP storage stores any prepended message from a forward
+ * as a separate file from the rest of the message
+ */
+ if (!ast_strlen_zero(vms.introfn) && ast_fileexists(vms.introfn, NULL, NULL) > 0) {
+ wait_file(chan, &vms, vms.introfn);
+ }
#endif
- if (cb) {
- cb(chan, vms.fn, duration);
- } else if ((wait_file(chan, &vms, vms.fn)) < 0) {
- ast_log(AST_LOG_WARNING, "Playback of message %s failed\n", vms.fn);
- } else {
- res = 0;
- }
-
- /* cleanup configs and msg */
- DISPOSE(vms.curdir, vms.curmsg);
- }
+ if (cb) {
+ cb(chan, vms.fn, duration);
+ } else if ((wait_file(chan, &vms, vms.fn)) < 0) {
+ ast_log(AST_LOG_WARNING, "Playback of message %s failed\n", vms.fn);
+ } else {
+ res = 0;
+ }
+
+ vms.heard[vms.curmsg] = 1;
+
+ /* cleanup configs and msg */
+ DISPOSE(vms.curdir, vms.curmsg);
play2_msg_cleanup:
if (vmu && open) {
Modified: team/mmichelson/trunk-digiumphones/include/asterisk/app_voicemail.h
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/trunk-digiumphones/include/asterisk/app_voicemail.h?view=diff&rev=361609&r1=361608&r2=361609
==============================================================================
--- team/mmichelson/trunk-digiumphones/include/asterisk/app_voicemail.h (original)
+++ team/mmichelson/trunk-digiumphones/include/asterisk/app_voicemail.h Fri Apr 6 17:05:07 2012
@@ -135,7 +135,6 @@
* \brief from_folder The folder from which the message is being forwarded
* \brief to_mailbox The mailbox to forward the message to
* \brief to_context The voicemail context of the to_mailbox
- * \brief to_folder The folder to which the message is being forwarded
* \brief num_msgs The number of messages being forwarded
* \brief msg_ids The message IDs of the messages in from_mailbox to forward
* \brief delete_old If non-zero, the forwarded messages are also deleted from from_mailbox.
@@ -149,7 +148,6 @@
const char *from_folder,
const char *to_mailbox,
const char *to_context,
- const char *to_folder,
size_t num_msgs,
int *msg_ids,
int delete_old);
More information about the svn-commits
mailing list