[Home]

Summary:ASTERISK-06465: Privacy Manager does not work unless caller presses '#'.
Reporter:Dan Swartzendruber (dswartz)Labels:
Date Opened:2006-03-03 08:52:45.000-0600Date Closed:2006-05-01 21:25:40
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/app_privacy
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_privacy.c.diff
Description:I was torn whether to call this minor or major.  Yes, there is a workaround, but there is no reasonable way for callers to know it.  Allison prompts them to enter their phone number, starting with the area code.  The code in question passes in a buffer 30 bytes long, so pressing 10 digits will NOT exit with a zero code, but rather, after the requisite number of seconds, ast_readstring() will time out and return the (documented) value 1.  This causes the subsequent loop to misbehave, because it is checking '!res').  Patch enclosed...



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

--- app_privacy.c.orig  2006-03-03 09:48:54.000000000 -0500
+++ app_privacy.c       2006-03-03 09:48:17.000000000 -0500
@@ -182,6 +182,8 @@
                       if (res < 0)
                               break;

+                       res = 0; /* timeout is success */
+
                       /*Make sure we get at least digits*/
                       if (strlen(phone) >= minlength )
                               break;
Comments:By: Dan Swartzendruber (dswartz) 2006-03-06 09:41:19.000-0600

I have faxed a disclaimer over the weekend.
By: Patrick Himebrook (thuper) 2006-03-06 16:53:49.000-0600

What were the configuration file settings that would cause this?
By: Dan Swartzendruber (dswartz) 2006-03-06 17:17:42.000-0600

I'm not sure I understand.  This isn't related to configuration settings.  The buffer passed in is 30 characters, so no 10-digit DTMF input will cause a zero return, e.g. it always returns a 1.  I thought this worked at some point in the past, but reading the code (both app_privacy.c and ast_readstring()), I don't see how it could?
By: Chris Tracy (adiemus) 2006-03-08 01:12:31.000-0600

Just wanted to report that this bug still exists in 1.2.5 and that the proposed patch here seems to fix the issue for me in 1.2.5.
By: Russell Bryant (russell) 2006-03-21 16:19:07.000-0600

Perhaps we should modify the privacy-prompt sound file to tell the user to press the '#' key when finished.
By: Philippe Lindheimer (p_lindheimer) 2006-04-14 13:27:05

[SEE MY CORRECTION BELOW ABOUT DSWARTZ's PROPOSED CHANGE, I THINK I WAS MISTAKEN. HOWEVER - SEE MY LATER NOTE WITH THE CALLER PRESENCE CHANGE NECESSARY)

I don't think the proposed change is correct. The return of a -1 implies an error. The return from a timeout returns a 1. Given this, I believe the proper fix for this is to check for >=0 as described in this patch. I generated this patch by modifiying the svn trunk for 1.2. I applied the same change to 1.2.4 which I am running and the change fixes the problem.

Concerning the discussion on this thread:

"Perhaps we should modify the privacy-prompt sound file to tell the user to press the '#' key when finished."

in my opinion that would not be adequate (although still a good addition). The code logic is flawed in the current implementation. It correctly falls out of the loop once they have put in an adequate number of digits but the subsequent if statement is flawed in looking for !res when it should be res >=0 since the maxretries will make sure it still fails if they didn't put in enough digits. I believe my suggestion is what was originally intended in the code flow.

--- app_privacy.c.orig  2006-04-14 10:59:42.000000000 -0700
+++ app_privacy.c       2006-04-14 11:00:40.000000000 -0700
@@ -189,7 +189,7 @@
               }

               /*Got a number, play sounds and send them on their way*/
-               if ((retries < maxretries) && !res ) {
+               if ((retries < maxretries) && res >= 0 ) {
                       res = ast_streamfile(chan, "privacy-thankyou", chan->language);
                       if (!res)
                               res = ast_waitstream(chan, "");



Following is the same patch applied to 1.2.4:


Index: app_privacy.c
===================================================================
--- app_privacy.c       (revision 46)
+++ app_privacy.c       (working copy)
@@ -193,7 +193,7 @@
               }

               /*Got a number, play sounds and send them on their way*/
-               if ((retries < maxretries) && !res ) {
+               if ((retries < maxretries) && res >= 0 ) {
                       res = ast_streamfile(chan, "privacy-thankyou", chan->language);
                       if (!res)
                               res = ast_waitstream(chan, "");


By: Philippe Lindheimer (p_lindheimer) 2006-04-14 18:59:08

I don't know if I should put this new information here or open a new bug. Here is my new twist:

Configuration: Asterisk 1.2.5
Call come in on a T1 PRI line:

Scenario 1:

Call comes in with CID, Privacy Manager checks the call and it is ok. The call proceeds and is routed to a Polycom SIP phone. The SIP invite goes out with the caller id and the receiving party sees the CID fine.

Scenario 2:

Call comes in using *67, Privacy Manager checks the call and requests the number. The number is entered. The call proceeds with the newely entered callerid. It is present in the channel variables and gets set in the CDR. However, the invite to the phone goes out as 'unknown' in the SIP message the the caller does not see the CID identity.

I have thoroughly investigated on this particular system. I can manually reset the callerid manually, even hard code it after the privacy manager and the SIP invite to the phone still shows unknown. In fact I did the following scenario:

1. Hardcode the CallerID right after the call to Pricacy Manager.
2. Make a normal call: result, the hard coded CID gets sent out in the SIP Invite
3. Make a *67 call: result, the SIP invite goes out with unknown

I have instrumented app_privacy.c with additonal debugs to examine the caller id fields as well as the ani and ani2 fields to determine if they were causing the problems. I have enven modified it to set the ani field to the same as the caller id phone number thinking that it may have been sending undefined because of the ani field, but this didn't fix it either.

I'm kind of at and stalling point at this point as I can't justify the expense of my client to continue digging into this issue, but the moral is this:

1. If a call comes in with a CID and passes through Pricacy Manager, everything is fine. And you can keep the CID or alter it - they are all reflected when the call is passed out in the SIP invite.

2. If a call comes in blocked (*67) and passes through Pricacy Manager, Pricacy Manager's setting of the CID, as well as any subsequent dialplan manipulation of the CID, is all reflected in the Channel variables and subsequently in the CDR records, however the SIP invites continue to go out as unknown, even after having modified to app_privacy.c to try and set the ani as well. (Not the ani2, but I verified that it comes in a 0 with a log message, whether or not the call is blocked).

p.s. - I think I may have mis-read dswartz's patch in my earlier comment - when I first looked at it I was seeing it as resetting a -1 to 0 (thought it was in the if clause). So ... that fix is probably fine as well. (too tired, too late last night diggin into this...)
By: Philippe Lindheimer (p_lindheimer) 2006-04-15 00:19:32

OK - Solved. When the calls come in on the PRI line they are set to a callingpres of 67 (decimal) which is never changed in Privacy Manager. You can work around this by calling SetCallerPres(allowed_passed_screen) after exiting from the privacy manager BUT this really should be the job of Pricacy Manager (or it will be the source of enless headaches for other people over time).

So, the following line can be added. I won't put a patch statement here since my current code is all cluttered up with debug messages that I now need to clean up. But it is ver straight forward:

         ast_set_callerid (chan, phone, "Privacy Manager", NULL);
==NEW==>  chan->cid.cid_pres = 1; /* set to allowed_passed_screen */ <==NEW==

Or maybe you actually want to do:

chan->cid.cid_pres &= 0xbf; /* assuming that is the right bit to mask */
(in my case I was getting 0x03 on un-blocked calls and 0x43 on blocked calls, the 0x43 did not let the CID out on the SIP invite, 0x01 and 0x03 did)

OK: HERE IS A PATCH FILE THAT DOES ALL MY CHANGES (result >=0 and clear the unavailable bit if set in callpres from PRI, leaving the other bits as is) One comment to make though concering this Privacy Manager. The only check that is used to determine if it needs to be entered is if the callerid field is set or not. Is it possible for a blocked call to come in with the caller id set but the PRI bit set to unavailable? (In which case - the Privacy Manager will exit doing nothing)? Anyhow, here's the patch:


Index: app_privacy.c
===================================================================
--- app_privacy.c       (.../packages/asterisk/current/apps/app_privacy.c)      (revision 6)
+++ app_privacy.c       (.../itb/trunk/atengo/packages/asterisk/asterisk/apps/app_privacy.c)    (working copy)
@@ -193,13 +193,21 @@
               }

               /*Got a number, play sounds and send them on their way*/
-               if ((retries < maxretries) && !res ) {
+               if ((retries < maxretries) && res >= 0 ) {
                       res = ast_streamfile(chan, "privacy-thankyou", chan->language);
                       if (!res)
                               res = ast_waitstream(chan, "");
-                       ast_set_callerid (chan, phone, "Privacy Manager", NULL);
-                       if (option_verbose > 2)
-                               ast_verbose (VERBOSE_PREFIX_3 "Changed Caller*ID to %s\n",phone);
+
+                       ast_set_callerid (chan, phone, "Privacy Manager", NULL);
+
+                       /* Clear the unavailable presence bit so if it came in on PRI
+                        * the caller id will now be passed out to other channels
+                        */
+                       chan->cid.cid_pres &= (AST_PRES_UNAVAILABLE ^ 0xFF);
+
+                       if (option_verbose > 2) {
+                               ast_verbose (VERBOSE_PREFIX_3 "Changed Caller*ID to %s, callerpres to %d\n",phone,chan->cid.cid_pres);
+                       }
                       pbx_builtin_setvar_helper(chan, "PRIVACYMGRSTATUS", "SUCCESS");
               } else {
                       if (priority_jump || option_priority_jumping)


By: Philippe Lindheimer (p_lindheimer) 2006-04-17 19:08:42

Russel,

sorry to ping you if you are already monitoring this. I ran into this same bug that dswartz ran into but with an added twist. I have documented it all in the bug instead of creating a new bug since it is all related. The net of it is, in addition to the bug something needs to be done with the unavailble callerpres PRI bit if it came in on PRI otherwise the call still gets sent to SIP phones (the only case I tested) as unavailble even though a CID has been obtained. I added a patch for that

It does open one more question though, in addition to checking for the presence of a Caller ID field, should the Privacy manager always clear this bit and/or run the caller through the Privacy Manager if the bit is set?

thanks for taking a look at this. It came up as an improtant feature for my Client and they would like to see the/a fix rolled into the code base so we don't have to track this separately.

thanks

p_lindheimer
By: Tilghman Lesher (tilghman) 2006-04-18 01:32:25

Please upload your patch as a separate file, not pasted into a bugnote.
By: Philippe Lindheimer (p_lindheimer) 2006-04-18 23:06:17

I uploaded the patch per your request.

Corydon - I noticed this is still marked for feedback, I added a patch a well ago. It fixes the problem including the PRI presence bits. Any chance this will get closed and roled in for the next release so we don't have to carry it along for our customer? thanks!


By: Tilghman Lesher (tilghman) 2006-05-01 21:18:28

Committed to 1.2, as of revision 24097.
By: Tilghman Lesher (tilghman) 2006-05-01 21:25:40

Merged to trunk, revision 24098.