[Home]

Summary:ASTERISK-25868: Sorcery "append to category" should allow filters
Reporter:Nick Repin (turnip)Labels:
Date Opened:2016-03-27 06:10:09Date Closed:2016-04-26 15:36:22
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/Configuration
Versions:13.7.2 13.8.0-rc1 Frequency of
Occurrence
Constant
Related
Issues:
is related toASTERISK-28563 Additional configuration [extName](+) not always working
Environment:N/AAttachments:
Description:Below is a typical FreePBX config with trunk "Callcentric", for which I am trying to add custom variable "forbidden_retry_interval". In other words, there are multiple categories with the same name ("Callcentric") but different types ("endpoint" vs "registration"), and one of the categories ("registration") needs to be appended.

==pjsip.conf==
{noformat}
...
#include pjsip.endpoint.conf
  ...
  [Callcentric]
  type=endpoint
  ...
...
#include pjsip.registration.conf
  [Callcentric]
  type=registration
  ...
...
#include pjsip.registration_custom_post.conf
  [Callcentric](+)
  ;type=registration  ; This doesn't help
  forbidden_retry_interval=600
{noformat}
==end==

The problem is, Sorcery searches for the first category named "Callcentric" and tries to append to it. In the above example, the first category is of type "endpoint" which does not accept "forbidden_retry_interval", so Sorcery displays an error. As long as Asterisk allows duplicate names for categories of different types, it should allow appending to a specific category, otherwise it's a bug. In case of FreePBX, the bug makes it impossible to add custom variables to pjsip.conf for SIP trunks and extensions.  

The simplest fix is to change main/config.c as follows:

{noformat}
--original code --
  } else if (!strcasecmp(cur, "+")) {
     *cat = ast_category_get(cfg, catname, NULL);
--new code--
  } else if (cur[0] == '+') {
     const char* filter = cur[1] ? (cur+1) : NULL;
     *cat = ast_category_get(cfg, catname, filter);
---
{noformat}

The above will allow a custom filter like this:

{noformat}
[Callcentric](+type=registration)
forbidden_retry_interval=600
{noformat}

A full-blown fix needs more coding to allow the use of commas in the filter (since parsing is currently done with "strsep"), e.g:

{noformat}
[Category]("+RegexValue1=RegexName1,RegexValue2=RegexName2",!,parent1,parent2)
AppendedName=AppendedValue
{noformat}
Comments:By: Asterisk Team (asteriskteam) 2016-03-27 06:10:10.485-0500

The severity of this issue has been automatically downgraded from "Blocker" to "Major". The "Blocker" severity is reserved for issues which have been determined to block the next release of Asterisk. This severity can only be set by privileged users. If this issue is deemed to block the next release it will be updated accordingly during the triage process.

By: Asterisk Team (asteriskteam) 2016-03-27 06:10:10.903-0500

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].

By: George Joseph (gjoseph) 2016-03-27 23:57:18.691-0500

Nick,  Can you test the patch under "Gerrit Reviews" and make sure it does what you need?


By: Nick Repin (turnip) 2016-04-23 05:53:54.675-0500

George,

Thank you for the prompt patch and sorry for the slow reply. I've reviewed the fix before testing it:

{noformat}
while ((cur = strsep(&c, ","))) {
  if (!strcasecmp(cur, "!")) {
     (*cat)->ignored = 1;
  } else if (cur[0] == '+') {
     char *filter = NULL;
     if (cur[1] != ',') {
        filter = &cur[1];
     }
     *cat = category_get_sep(cfg, catname, filter, '&');
{noformat}

It seems to me that "if (cur[1] != ',') {" is incorrect because the condition is always true. There will never be a comma in the token because it'd be zeroed out by strsep in "while ((cur = strsep(&c, ","))) {"

I haven't tried the fix because of that.

Thanks,

Nick

By: George Joseph (gjoseph) 2016-04-23 17:44:52.519-0500

Yeah, that was left over from an earlier version of the patch.  It's harmless and will be corrected shortly.