Use of builtin CID lookup can trash valid incoming CID info

Please specify what version of FreePBX you are using. All I can say right now is that it works for me.

Freepbx 2.5.1.1 on top of asterisk 1.6.

Here is the problematic code:

[cidlookup]
include => cidlookup-custom
exten => cidlookup_5,1,Set(CALLERID(name)=${DB(cidname/${CALLERID(num)})})
exten => cidlookup_5,n,Return()
exten => cidlookup_return,1,Set(CALLERID(name)=${DB(cidname/${CALLERID(num)})})
exten => cidlookup_return,n,Return()

Rereading my description, I may not have been clear. My complaint is that it overwrites the CID name, not the CID info in general.

Okay, I have root-caused this as being broken in 1.6. Prior to that, LookupCIDName() called. That is documented as deprecated and we should do "${DB(cidname/${CALLERID(num)})}" instead (per asterisk source code comments). What the comments do not make clear enough is that the routine does NOT set the CIDNAME if the lookup failed from the ASTDB, whereas the naive assignment recommended will in fact trash an existing CIDNAME with a null string in the same case.

danswartz,

can you have a look at the attached patch. I don't have a 1.6 system but I believe this is the correct syntax to address your needs and you are right that we should not set it if it returns null. If it checks out I can check it in.

Index: extensions.class.php
===================================================================
--- extensions.class.php        (revision 7407)
+++ extensions.class.php        (working copy)
@@ -985,6 +985,7 @@
 
                if (version_compare($version, "1.6", "ge")) {
                        $outstr=addslashes('Set(CALLERID(name)=${DB(cidname/${CALLERID(num)})})');
+                       $outstr='ExecIf($["${DB(cidname/${CALLERID(num)}" != ""]?Set,CALLERID(name)=${DB(cidname/${CALLERID(num)})})';
                        return $outstr;
                } else { 
                        return "LookupCIDName";

A couple of syntax issues kept this from working. Here is what it should be (I believe). I made the below changes and it now works:

$outstr='ExecIf?($["${DB(cidname/${CALLERID(num)})" != ""]?Set(CALLERID(name)=${DB(cidname/${CALLERID(num)})}))';

e.g. a ',' after Set instead of '(' [is that legal?] and a missing ')' for the first call to DB?

Ok I may have been a little quick to plug that in, thanks for trying it out. I forgot that ExecIf? changes from ? to , AND from the other , to ().

Anyhow, I'm not sure I got it right for what you pasted in above because of the way trac likes to turn the execif into the ExceIf? wiki format, so let me paste another patch from what I think is correct and if it looks good, let me know and I'll check it in. Otherwise, paste your code between a starting {{{ and an ending }}} so we can see what it is.

Index: extensions.class.php
===================================================================
--- extensions.class.php        (revision 7407)
+++ extensions.class.php        (working copy)
@@ -984,8 +984,7 @@
                global $version;
 
                if (version_compare($version, "1.6", "ge")) {
-                       $outstr=addslashes('Set(CALLERID(name)=${DB(cidname/${CALLERID(num)})})');
-                       return $outstr;
+                       return 'ExecIf($["${DB(cidname/${CALLERID(num)})}" != ""]?Set(CALLERID(name)=${DB(cidname/${CALLERID(num)})}))';
                } else { 
                        return "LookupCIDName";
                }

That did it, thanks!

(In [7482]) fixes #3469 - don't set CNAM when calling LookupCIDName and null is returned, Asterisk 1.6+ only

(In [7545]) Merged revisions 7461-7519,7521,7524-7544 via svnmerge from http://svn.freepbx.org/freepbx/branches/2.5

........

r7461 | mickecarlsson | 2009-02-16 12:48:05 -0800 (Mon, 16 Feb 2009) | 1 line

Closes #3540 adds missing eclosures for localization in ari. Thank you chocho

........

r7482 | p_lindheimer | 2009-03-12 06:18:05 -0700 (Thu, 12 Mar 2009) | 1 line

fixes #3469 - don't set CNAM when calling LookupCIDName and null is returned, Asterisk 1.6+ only

........

r7530 | p_lindheimer | 2009-03-15 08:38:55 -0700 (Sun, 15 Mar 2009) | 1 line

fixes #3588 - always move fw_langpack to be the last module to process

........

r7544 | p_lindheimer | 2009-03-21 08:49:07 -0700 (Sat, 21 Mar 2009) | 1 line

fixes #3592 clear output arrays to exec before calling since exec appends data

........