Summary: | ASTERISK-23035: ConfBridge with name longer than max (32 chars) results in several bridges with same conf_name | ||
Reporter: | Iñaki Cívico (icivico) | Labels: | |
Date Opened: | 2013-12-19 08:57:16.000-0600 | Date Closed: | 2014-06-04 02:28:52 |
Priority: | Minor | Regression? | |
Status: | Closed/Complete | Components: | Applications/app_confbridge |
Versions: | 12.0.0-beta2 | Frequency of Occurrence | |
Related Issues: | |||
Environment: | Attachments: | ( 0) confbridge-enforce_max-1.8.patch ( 1) confbridge-enforce_max-11up.patch | |
Description: | This simple dialplan is used to reproduce issue:
{noformat} exten => _X.,1,ConfBridge(${confid}) {noformat} where confid is longer than MAX_CONF_NAME (apps/confbridge/include/confbridge.h) Each time a call arrives to this point, a new bridge is created with same name (confid truncated to MAX_CONF_NAME length) but different uuid. In app_confbridge/join_conference_bridge, ao2_find() searches with full conference name (longer than max length). ao2_find result is always null as bridge has a truncated name, then a new bridge is created with the same name. Behavior is extrange resulting in a several calls connected to the same conf but not earing each other. Here is a console trace: {noformat} CLI> confbridge list Conference Bridge Name Users Marked Locked? ================================ ====== ====== ======== 7e012c4d-9f34-4628-a483-c152ad5 1 0 unlocked 7e012c4d-9f34-4628-a483-c152ad5 1 0 unlocked {noformat} | ||
Comments: | By: Matt Jordan (mjordan) 2013-12-19 09:50:08.391-0600 Well... it isn't valid to create a ConfBridge with a name longer than the maximum length. The only solution I would advocate here would be to reject anything that attempts to create a ConfBridge with a name that is too long. By: Iñaki Cívico (icivico) 2013-12-19 09:55:34.171-0600 Yes, I think that should be enough. Maybe a note about max length in command documentation? Thanks! By: Rusty Newton (rnewton) 2013-12-19 18:58:29.114-0600 +1 Yeah if there is a maximum, we should enforce it rather than let strange unexpected behavior happen when the user exceeds the max. By: Corey Farrell (coreyfarrell) 2013-12-20 13:23:02.053-0600 AST_MAX_EXTENSION is defined as 80, it seems inconsistent/wrong for MAX_CONF_NAME to be 32. In my opinion the following should never produce a conference name that is too long: exten => _X.,1,ConfBridge($\{EXTEN\}) I say this on the assumption that AST_MAX_EXTENSION is consistently enforced. By: Matt Jordan (mjordan) 2013-12-20 13:33:55.052-0600 I'm fine with AST_MAX_EXTENSION == MAX_CONF_NAME. By: Corey Farrell (coreyfarrell) 2013-12-26 19:44:09.195-0600 Patch to reject conference name's that are too long. The storage size has been increased to AST_MAX_EXTENSION. Also adds protection against an odd error case: same => n,ConfBridge($\{EMPTY_VAR\},default) Instead of checking for empty 'data' the updated code checks for empty first parameter. I've compiled with this in 1.8/11/12. Runtime testing was done with 11. By: Iñaki Cívico (icivico) 2013-12-30 10:49:14.701-0600 Succesfully tested with release 12.0.0 By: Iñaki Cívico (icivico) 2014-05-29 08:37:13.823-0500 Patch working fine in 12.2.0 By: Matt Jordan (mjordan) 2014-05-29 12:36:04.252-0500 I'd say this patch is good to go. |