[Home]

Summary:ASTERISK-13691: searchcontexts=yes causes voicemail boxes to be setup wrong
Reporter:Leif Madsen (lmadsen)Labels:
Date Opened:2009-03-03 17:36:05.000-0600Date Closed:2009-03-05 13:27:09.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 14599.patch
Description:In issue 13853 I thought the following note was an issue with IMAP, but it seems it is an issue regardless of whether you're using IMAP. The issue comes from the searchcontext=yes option being enabled. The text in the note I posted to issue ASTERISK-13028 is still relevant.

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

I can't load more than a single user with the same name in the voicemail.conf, even if they are under different VM contexts. Here is the output I get with 3 different contexts and users:

*CLI> voicemail show users
Context Mbox User Zone NewMsg
[Mar 3 14:00:32] WARNING[1681]: app_voicemail.c:1190 messagecount: IMAP user not set for mailbox general
default general New User -1
default 1000 Leif Madsen Default 466474496
xyz 1001 Leif Madsen 466474496
abc 1002 Leif Madsen 466474496


But if I change the 1002 user in the [abc] context to 1001 (under the same context) I get:

*CLI> voicemail show users
Context Mbox User Zone NewMsg
[Mar 3 14:01:22] WARNING[1722]: app_voicemail.c:1190 messagecount: IMAP user not set for mailbox general
default general New User -1
default 1000 Leif Madsen Default 318031296
xyz 1001 Leif Madsen 318031296


My configuration follows:

imapserver = localhost
imapport = 143
imapflags = novalidate-cert
format = wav
searchcontexts = yes
expungeonhangup = yes
listen-control-forward-key = 3
listen-control-reverse-key = 1
listen-control-pause-key = 2
listen-control-restart-key = 0
listen-control-stop-key = 456789*#
sendvoicemail = yes
review = yes
authuser = lmadsen
authpassword = XXXXXXXX

[default]
1000 => 1000,Leif Madsen Default,,,imapuser=lmadsen,imappassword=XXXXXX

[xyz]
1001 => 1000,Leif Madsen,,,imapuser=lmadsen,imappassword=XXXXXX

[abc]
1001 => 1000,Leif Madsen,,,imapuser=lmadsen,imappassword=XXXXXX
Comments:By: Mark Michelson (mmichelson) 2009-03-03 17:56:13.000-0600

The note lmadsen refers to on issue ASTERISK-13028 is ~101102
By: Mark Michelson (mmichelson) 2009-03-04 09:07:12.000-0600

So, I think the problem here isn't so much that the code is broken, but that searchcontexts is not documented well/properly.

Here's a breakdown, first of all, of what searchcontexts accomplishes. If you wrote a dialplan that called VoiceMailMain(2000), then you are going to call into VoiceMailMain for mailbox 2000, but for what context? Without searchcontexts on, we assume the "default" context. With searchcontexts on, then we will search through the mailbox contexts defined in voicemail.conf for mailbox 2000.

Now for searchcontexts to be able to work, it means that you can't possibly define the same mailbox in two different contexts. Otherwise, you're leaving things up to chance which mailbox you would be calling into in the example VoiceMailMain call above. app_voicemail attempts to correct for any ambiguities you may have in your config file by only allowing for a single mailbox of any given number to exist, even if you have specified the same mailbox in multiple voicemail contexts.

The problem with app_voicemail's actions is that it is not clear just from "voicemail show users" output whether it is discarding an earlier mailbox declaration when it comes across a duplicate or if it is ignoring the new one when it is encountered. The output from the CLI command almost makes it look as though it did some weird merge operation between the mailboxes. This needs to be corrected. Furthermore, I think that there should be a big fat warning issued if you attempt to define the same mailbox name in two different contexts if using the searchcontexts option.
By: Mark Michelson (mmichelson) 2009-03-04 13:11:13.000-0600

I've uploaded 14599.patch.

There are a couple of provisions made in this patch.

1. If you have searchcontexts turned on and we read a duplicated mailbox name from the config file, then we will issue a notice saying that we ignored the option. Furthermore, if the duplicate mailbox names are in different contexts, a larger warning message will be printed alerting them that they have an ambiguous configuration file and that it needs to be changed.

2. A similar ignore check was also added for the case where you do not have searchcontexts turned on. The old behavior was to silently drop the first of the duplicated entries. Now a notice is issued stating that the second duplicate in the config is being ignored.

3. The voicemail.conf.sample file now has a clarification in the searchcontexts documentation stating that a requirement of using the option is to have unique mailbox names across all contexts.

Let me know what you think about these. As I'm writing this, I'm wondering if the notices printed should actually be warnings instead. I think I may also need to make a similar change for loading mailboxes from realtime.
By: Leif Madsen (lmadsen) 2009-03-05 12:38:03.000-0600

First:

[Mar  5 13:25:34] WARNING[13031]: app_voicemail.c:7908 find_or_create: It has been detected that you have defined mailbox '1000' in separate
       contexts and that you have the 'searchcontexts' option on. This type of
       configuration creates an ambiguity that you likely do not want. Please
       amend your voicemail.conf file to avoid this situation.

The formatting may not show up right here... but I would suggest starting the "It has been detected..." on a new line so that the formatting looks a little cleaner.


-- I've done a lot of testing of various things here, and other than my suggestion above, all of this looks great. I'd probably change the NOTICE to a WARNING as well though as it is telling you of unexpected behaviour that you should take action on. I believe NOTICE is more of a, "hey, this happened - nothing to worry about".
By: Mark Michelson (mmichelson) 2009-03-05 12:40:04.000-0600

All right. I'll upload a new patch pronto!
By: Mark Michelson (mmichelson) 2009-03-05 12:47:51.000-0600

Actually, since all I had to do was add an extra new line and change the notices to warnings, I'm just going to commit the new patch and not bother uploading it.
By: Digium Subversion (svnbot) 2009-03-05 12:58:49.000-0600

Repository: asterisk
Revision: 180380

U   branches/1.4/apps/app_voicemail.c
U   branches/1.4/configs/voicemail.conf.sample

------------------------------------------------------------------------
r180380 | mmichelson | 2009-03-05 12:58:49 -0600 (Thu, 05 Mar 2009) | 25 lines

Fix broken mailbox parsing when searchcontexts option is enabled

When using the searchcontexts option in voicemail.conf, the code
made the assumption that all mailbox names defined were unique across
all contexts. However, the code did nothing to actually enforce this
assumption, nor did it do anything to alert a user that he may have
created an ambiguity in his voicemail.conf file by defining the same
mailbox name in multiple contexts.

With this change, we now will issue a nice long warning if searchcontexts
is on and we encounter the same mailbox name in multiple contexts and ignore
any duplicates after the first box. Whether searchcontexts is enabled or not,
if we come across a duplicate mailbox in the same context, then we will issue
a warning and ignore the duplicated mailbox. I have also added a small note
to voicemail.conf.sample in the explanation for searchcontexts explaining
that you cannot define the same mailbox in multiple contexts if you have
enabled the option.

(closes issue ASTERISK-13691)
Reported by: lmadsen
Patches:
     14599.patch uploaded by mmichelson (license 60) (with slight modification)
Tested by: lmadsen


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

http://svn.digium.com/view/asterisk?view=rev&revision=180380
By: Digium Subversion (svnbot) 2009-03-05 13:00:36.000-0600

Repository: asterisk
Revision: 180380

U   branches/1.4/apps/app_voicemail.c
U   branches/1.4/configs/voicemail.conf.sample

------------------------------------------------------------------------
r180380 | mmichelson | 2009-03-05 12:58:48 -0600 (Thu, 05 Mar 2009) | 25 lines

Fix broken mailbox parsing when searchcontexts option is enabled.

When using the searchcontexts option in voicemail.conf, the code
made the assumption that all mailbox names defined were unique across
all contexts. However, the code did nothing to actually enforce this
assumption, nor did it do anything to alert a user that he may have
created an ambiguity in his voicemail.conf file by defining the same
mailbox name in multiple contexts.

With this change, we now will issue a nice long warning if searchcontexts
is on and we encounter the same mailbox name in multiple contexts and ignore
any duplicates after the first box. Whether searchcontexts is enabled or not,
if we come across a duplicate mailbox in the same context, then we will issue
a warning and ignore the duplicated mailbox. I have also added a small note
to voicemail.conf.sample in the explanation for searchcontexts explaining
that you cannot define the same mailbox in multiple contexts if you have
enabled the option.

(closes issue ASTERISK-13691)
Reported by: lmadsen
Patches:
     14599.patch uploaded by mmichelson (license 60) (with slight modification)
Tested by: lmadsen


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

http://svn.digium.com/view/asterisk?view=rev&revision=180380
By: Digium Subversion (svnbot) 2009-03-05 13:14:15.000-0600

Repository: asterisk
Revision: 180383

_U  trunk/
U   trunk/apps/app_voicemail.c
U   trunk/configs/voicemail.conf.sample

------------------------------------------------------------------------
r180383 | mmichelson | 2009-03-05 13:14:14 -0600 (Thu, 05 Mar 2009) | 31 lines

Merged revisions 180380 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r180380 | mmichelson | 2009-03-05 12:58:48 -0600 (Thu, 05 Mar 2009) | 25 lines
 
 Fix broken mailbox parsing when searchcontexts option is enabled.
 
 When using the searchcontexts option in voicemail.conf, the code
 made the assumption that all mailbox names defined were unique across
 all contexts. However, the code did nothing to actually enforce this
 assumption, nor did it do anything to alert a user that he may have
 created an ambiguity in his voicemail.conf file by defining the same
 mailbox name in multiple contexts.
 
 With this change, we now will issue a nice long warning if searchcontexts
 is on and we encounter the same mailbox name in multiple contexts and ignore
 any duplicates after the first box. Whether searchcontexts is enabled or not,
 if we come across a duplicate mailbox in the same context, then we will issue
 a warning and ignore the duplicated mailbox. I have also added a small note
 to voicemail.conf.sample in the explanation for searchcontexts explaining
 that you cannot define the same mailbox in multiple contexts if you have
 enabled the option.
 
 (closes issue ASTERISK-13691)
 Reported by: lmadsen
 Patches:
       14599.patch uploaded by mmichelson (license 60) (with slight modification)
 Tested by: lmadsen
........

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

http://svn.digium.com/view/asterisk?view=rev&revision=180383
By: Digium Subversion (svnbot) 2009-03-05 13:21:24.000-0600

Repository: asterisk
Revision: 180404

_U  branches/1.6.0/
U   branches/1.6.0/apps/app_voicemail.c
U   branches/1.6.0/configs/voicemail.conf.sample

------------------------------------------------------------------------
r180404 | mmichelson | 2009-03-05 13:21:24 -0600 (Thu, 05 Mar 2009) | 38 lines

Merged revisions 180383 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r180383 | mmichelson | 2009-03-05 13:14:14 -0600 (Thu, 05 Mar 2009) | 31 lines
 
 Merged revisions 180380 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r180380 | mmichelson | 2009-03-05 12:58:48 -0600 (Thu, 05 Mar 2009) | 25 lines
   
   Fix broken mailbox parsing when searchcontexts option is enabled.
   
   When using the searchcontexts option in voicemail.conf, the code
   made the assumption that all mailbox names defined were unique across
   all contexts. However, the code did nothing to actually enforce this
   assumption, nor did it do anything to alert a user that he may have
   created an ambiguity in his voicemail.conf file by defining the same
   mailbox name in multiple contexts.
   
   With this change, we now will issue a nice long warning if searchcontexts
   is on and we encounter the same mailbox name in multiple contexts and ignore
   any duplicates after the first box. Whether searchcontexts is enabled or not,
   if we come across a duplicate mailbox in the same context, then we will issue
   a warning and ignore the duplicated mailbox. I have also added a small note
   to voicemail.conf.sample in the explanation for searchcontexts explaining
   that you cannot define the same mailbox in multiple contexts if you have
   enabled the option.
   
   (closes issue ASTERISK-13691)
   Reported by: lmadsen
   Patches:
         14599.patch uploaded by mmichelson (license 60) (with slight modification)
   Tested by: lmadsen
 ........
................

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

http://svn.digium.com/view/asterisk?view=rev&revision=180404
By: Digium Subversion (svnbot) 2009-03-05 13:27:08.000-0600

Repository: asterisk
Revision: 180425

_U  branches/1.6.1/
U   branches/1.6.1/apps/app_voicemail.c
U   branches/1.6.1/configs/voicemail.conf.sample

------------------------------------------------------------------------
r180425 | mmichelson | 2009-03-05 13:27:08 -0600 (Thu, 05 Mar 2009) | 38 lines

Merged revisions 180383 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r180383 | mmichelson | 2009-03-05 13:14:14 -0600 (Thu, 05 Mar 2009) | 31 lines
 
 Merged revisions 180380 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r180380 | mmichelson | 2009-03-05 12:58:48 -0600 (Thu, 05 Mar 2009) | 25 lines
   
   Fix broken mailbox parsing when searchcontexts option is enabled.
   
   When using the searchcontexts option in voicemail.conf, the code
   made the assumption that all mailbox names defined were unique across
   all contexts. However, the code did nothing to actually enforce this
   assumption, nor did it do anything to alert a user that he may have
   created an ambiguity in his voicemail.conf file by defining the same
   mailbox name in multiple contexts.
   
   With this change, we now will issue a nice long warning if searchcontexts
   is on and we encounter the same mailbox name in multiple contexts and ignore
   any duplicates after the first box. Whether searchcontexts is enabled or not,
   if we come across a duplicate mailbox in the same context, then we will issue
   a warning and ignore the duplicated mailbox. I have also added a small note
   to voicemail.conf.sample in the explanation for searchcontexts explaining
   that you cannot define the same mailbox in multiple contexts if you have
   enabled the option.
   
   (closes issue ASTERISK-13691)
   Reported by: lmadsen
   Patches:
         14599.patch uploaded by mmichelson (license 60) (with slight modification)
   Tested by: lmadsen
 ........
................

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

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