Summary:ASTERISK-05184: [patch] [post 1.2] FILTER dialplan function
Reporter:Carlos Antunes (cmantunes)Labels:
Date Opened:2005-09-28 21:32:42Date Closed:2005-12-23 14:12:10.000-0600
Versions:Frequency of
Environment:Attachments:( 0) 20051004__bug5327.diff.txt
( 1) func_sanitize.c
Description:Recently on asterisk-users mailing list, someone asked for a way to clean, or sanitize, phone numbers returned from a database or LDAP server. Someone suggested using AGI and a Perl script. But this is the kind of function that I believe m,akes sense to have builtin. As such, I present to the world the SANITIZE_PHONE_NUMBER dialplan function. If the gods are truly good, this will be merged into CVS HEAD. Let me know what you think.


(NOTE: I'm not sure whetrher the fax with the disclaimer made it or not. Who can confirm?)
Comments:By: Tilghman Lesher (tilghman) 2005-09-29 11:35:18

I'd go with something a little more generic, like

What you have here is a very specific implementation of a very specific need.  Abstracting it away from your needs makes it generally useful (shorter, too!)

By: Carlos Antunes (cmantunes) 2005-09-30 09:53:19

Corydon: the examples you provided do not actually mimic the action of SANITIZE_PHONE_NUMBER. When the argument supplied is in Caller Id format ("9th National" <1-800-123-4567>), your example would return 918001234567. However, SANITIZE_PHONE_NUMBER would return 18001234567. It is certainly possible to write a more complex regular expression but the thing with regular expressions is that they are computationally expensivem at least in relative terms. What do you think?

By: Tilghman Lesher (tilghman) 2005-09-30 10:11:23

Note that I specified CALLERID(num), not CALLERID(all).  Try it before putting your foot in your mouth again.

By: Russell Bryant (russell) 2005-09-30 10:24:12

I would vote for FILTER(), since GREP() might lead people to believe it has all of the same functionality as /bin/grep ...

By: Carlos Antunes (cmantunes) 2005-09-30 11:41:56

Corydon: if my understanding of how CALLERID(num) works is correct, it actually reads/writes the caller id number from/to the channel. However, that's not the intended purpose of the sanitize function. The purpose is to obtain a string with just digits from whatever source, being * DB, a Mysql server or a LDAP server and do with that string whatever one wants, but not necessarly set the channel CallerId. Regarding my puting the foot on my mouth, I do it all the time. You really don't need to tell me that.

By: Tilghman Lesher (tilghman) 2005-09-30 12:29:42

FILTER function diff uploaded.  Disclaimer on file.

By: Carlos Antunes (cmantunes) 2005-09-30 13:36:49

I'd like to point out that, while FILTER is most definitely useful function, it doesn't know how to extract a phone number from a string in CallerID format. Going back to my earlier example, FILTER(0123456789, "9th National" <1-800-123-4567>) returns 918001234567 while SANITIZE_PHONE_NUMBER("9th National" <1-800-123-4567>)  returns 18001234567.

By: Tilghman Lesher (tilghman) 2005-09-30 14:06:45

Try again:

Set(CALLERID(all)=9th National Bank <18001234567>)

sets foo to 18001234567


sets foo to 918001234567

Do you see the (subtle) difference between these two invocations?

By: Carlos Antunes (cmantunes) 2005-09-30 14:35:23

Corydon: I understand what you are doing. What I don't understand is why you'd want to have to set the caller id on the channel when what I was trying to accomplish has nothing to do with the caller id on the channel. If a user wants to sanitize a phone number (unrelated to the caller id on ther channel), the use with my function just does it. With your approach, the use as to save the current caller id, set the caller id to whatever he wants to sanitize, use the filter function, and finally, set the caller id on the channel back to the original value. Now, compare this with simply using SANITIZE_PHONE_NUMBER.

By: Tilghman Lesher (tilghman) 2005-09-30 14:37:03

Here's another function, which will split out an arbitrary specified callerid string, if specified, using the existing CALLERID function.

So this would make the compound structure as follows:

Set(opt=9th National <18001234567>)

By: Carlos Antunes (cmantunes) 2005-09-30 14:45:16

Corydon: just saw your patch to the CALLERID function. With that patch it makes sense to use the FILTER(CALLERID) combo.

By: Tilghman Lesher (tilghman) 2005-09-30 16:49:09

Yes; I am attempting to make it generic enough that it works across multiple situations, rather than the one specific situation that you have a need for.

By: Russell Bryant (russell) 2005-09-30 17:20:48

Corydon, check out my patch to your patch.  :)  I removed an unused variable, added a check to make sure we didn't go past the end of the buffer, and removed the internal for loop, even though strchr basically does the same thing your loop did.  Take what you want from it.  It was just some thoughts I had as I looked at it.

By: Tilghman Lesher (tilghman) 2005-09-30 17:42:19

Ah, good call.  I had based mine off of the source to strsep(3), and I hadn't thought that using strchr was essentially the same.  I also changed the second conditional that you had (another good catch) to something a little clearer.

By: Russell Bryant (russell) 2005-10-04 15:16:12

Shouldn't "buf + len > outbuf" be "buf + len > outbuf - 1" ?  Otherwise, the loop will terminate when outbuf == buf + len, which is one byte past the end of the buffer, and a '\0' will be written there.

By: Tilghman Lesher (tilghman) 2005-10-04 15:58:47

No, but it should be "buf + len > outbuf + 1" or "buf + len - 1 > outbuf".  Same logic is correct; you just have the direction wrong.

By: Russell Bryant (russell) 2005-10-04 17:11:49

D'oh!  Well, at least you understood what I meant.  :)

By: Tilghman Lesher (tilghman) 2005-10-04 22:10:04

Updated patch to deal with patch conflict.

By: Russell Bryant (russell) 2005-12-21 09:33:59.000-0600

I think that the arguments to CALLERID should be dup'd with ast_strdupa(data) since they are being modified by the function.  While we're at it, we should probably have it use the arg parsing macros.

By: Tilghman Lesher (tilghman) 2005-12-21 10:02:51.000-0600

It's superfluous to call ast_strdupa() inside a dialplan function, given that we've already called ast_strdupa() in pbx.c: ast_func_read() and ast_func_write().  There's no need to allocate additional stack space to process the arguments inside the actual function code.

By: Tilghman Lesher (tilghman) 2005-12-23 14:11:54.000-0600

FILTER and CALLERID optional argument committed to trunk.