| Summary: | ASTERISK-22982: CEL/cel_custom: Using non-standard func_channel CHANNEL(..) variables causes segfaults | ||||
| Reporter: | Walter Doekes (wdoekes) | Labels: | |||
| Date Opened: | 2013-12-13 04:40:02.000-0600 | Date Closed: | |||
| Priority: | Minor | Regression? | No | ||
| Status: | Open/New | Components: | CEL/cel_custom CEL/cel_sqlite3_custom | ||
| Versions: | 11.7.0 13.18.4 | Frequency of Occurrence | |||
| Related Issues: |
| ||||
| Environment: | Attachments: | ||||
| Description: | I'm confused here.
In {{cel_odbc}} (and {{cel_pgsql}}) I simply get a certain set of columns, like {{cid_name}}, {{accountcode}}, and so on. In {{cel_custom}} (and {{cel_sqlite}}) I have to specify channel functions, like {{$\{CALLERID(name)}}} and {{$\{CHANNEL(accountcode)}}}. Those will get run through ast_str_substitute_variables_full. -But, then the channel isn't completely initialized -- which it apparently isn't -- we can't get all possible values out of that:- But, since the channel is a dummy channel, we can't get all possible values out of that: For instance, using {{$\{CHANNEL(channeltype)}}} yields this: {noformat} Program received signal SIGSEGV, Segmentation fault. 445 locked_copy_string(chan, buf, ast_channel_tech(chan)->type, len); (gdb) print ast_channel_tech(chan) $1 = (const struct ast_channel_tech *) 0x0 (gdb) back #0 0x00007fffc73dc270 in func_channel_read (chan=0x7ffff0000b28, function=0x7ffff7fc7a00 "CHANNEL", data=0x7ffff7fc7a08 "channeltype", buf=0x7ffff00034b8 "", len=4096) at func_channel.c:445 #1 0x0000000000532c0d in ast_func_read2 (chan=0x7ffff0000b28, function=0x7ffff0003478 "CHANNEL(channeltype)", str=0x7ffff7fc7b38, maxlen=0) at pbx.c:4127 #2 0x00000000005331f1 in ast_str_substitute_variables_full (buf=0x7ffff7fc7c20, maxlen=0, c=0x7ffff0000b28, headp=0x0, templ=0x7ffff00021e3 "{UNIQUETREE}/${CHANNEL(channeltype)})", used=0x7ffff7fc7c30) at pbx.c:4256 #3 0x0000000000533170 in ast_str_substitute_variables_full (buf=0x7ffff7fc7d00, maxlen=0, c=0x7ffff0000b28, headp=0x0, templ=0xe9c85e "ype})},${CSV_QUOTE(${eventtime})},${CSV_QUOTE(${CALLERID(name)})},${CSV_QUOTE(${CALLERID(num)})},${CSV_QUOTE(${CALLERID(ANI)})},${CSV_QUOTE(${CALLERID(RDNIS)})},${CSV_QUOTE(${CALLERID(DNID)})},${CSV_Q"..., used=0x7ffff7fc7cc8) at pbx.c:4246 #4 0x000000000053373a in ast_str_substitute_variables (buf=0x7ffff7fc7d00, maxlen=0, chan=0x7ffff0000b28, templ=0xe9c84a "${CSV_QUOTE(${eventtype})},${CSV_QUOTE(${eventtime})},${CSV_QUOTE(${CALLERID(name)})},${CSV_QUOTE(${CALLERID(num)})},${CSV_QUOTE(${CALLERID(ANI)})},${CSV_QUOTE(${CALLERID(RDNIS)})},${CSV_QUOTE(${CALLE"...) at pbx.c:4343 #5 0x00007fffc77ec7e3 in custom_log (event=0x7fffc0032fb0, userdata=0x0) at cel_custom.c:149 #6 0x00000000004c8872 in handle_event (data=0x7fffc0033138) at event.c:1520 {noformat} Dereferencing 0x0 for 'type' is obviously bad. Same goes for {{ast_channel_zone(chan)}} and probably a bunch of others too. _The question_ Did we really want to call these functions? Or do we only want the {{$\{CSV_QUOTE()}}} expansion? If the former is the case, why don't I have access to my other channel variables? If the latter, why the detour through the CHANNEL and CALLERID functions. Couldn't we call the variables {{$\{cid_name}}} and {{$\{accountcode}}} instead? _Answering parts of my question_ Apparently we get a bogus channel that doesn't hold any more info than exactly the variables put in from the CEL event. {noformat} dummy = ast_cel_fabricate_channel_from_event(event); {noformat} Like I mentioned in the first line: this sets the wrong expectations and causes confusion. And it seems to me that calling the {{CALLERID}} and friends would cause more overhead than loading up the vars through a bunch of {{ast_var_assign}}'s. Is there a reason why we didn't do that in the first place? Cheers, Walter | ||||
| Comments: | By: Matt Jordan (mjordan) 2013-12-13 07:59:15.096-0600 Just curious - which event was it blowing up on? Ideally, even on a new channel event, the channel is supposed to be fully initialized before sending out the CEL event, just so that there's something there to query. As for why it's done this, way, I'm guessing it's because it's how the CDR engine does it for its adaptive backends. Probably nothing more than that. By: Walter Doekes (wdoekes) 2013-12-13 08:46:01.297-0600 Summarizing: 1. The dummy channel exists so we can do text substitution on it, like CSV_QUOTE(). 2. There doesn't seem to be a reason to prefer $\{CHANNEL(accountcode)} over $\{accountcode}. 3. Fixing this would require: (a) adding the appropriate vars and (b) deprecating the CHANNEL/CALLERID function calls. 4. After that you could still get things to crash by calling functions that manipulate other channel data than just variables, but you wouldn't be inclined to, since there are no recent examples that do so. (Perhaps when registering a function the registerer should specify whether it is safe to call it on a dummy channel: string functions get a yes, all functions that touch more than just variables get a no.) | ||||