[Asterisk-code-review] func_channel: Added new OTHER_CHANNEL function (asterisk[master])

[Mark Murawski]

Attention is currently required from: N A.
Mark Murawski has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/15893 )

Change subject: func_channel: Added new OTHER_CHANNEL function
......................................................................


Patch Set 7:

(1 comment)

Patchset:

PS7: 
This function has been needed for a very long time.

Here's one of, and the main good reason to push this:
The AMI Setvar uses identical flow:
 - Get Channel By Name
 - Set Variable


static int action_setvar(struct mansession *s, const struct message *m)
{
        struct ast_channel *c = NULL;
        const char *name = astman_get_header(m, "Channel");
        const char *varname = astman_get_header(m, "Variable");
        const char *varval = astman_get_header(m, "Value");
        int res = 0;

        if (ast_strlen_zero(varname)) {
                astman_send_error(s, m, "No variable specified");
                return 0;
        }

        if (!ast_strlen_zero(name)) {
                if (!(c = ast_channel_get_by_name(name))) {
                        astman_send_error(s, m, "No such channel");
                        return 0;
                }
        }

        res = pbx_builtin_setvar_helper(c, varname, S_OR(varval, ""));

        if (c) {
                c = ast_channel_unref(c);
        }
        if (res == 0) {
                astman_send_ack(s, m, "Variable Set");
        } else {
                astman_send_error(s, m, "Variable not set");
        }
        return 0;
}

There is nothing preventing a system interacting with AMI from setting a variable on an arbitrary channel.  What's the main reason for not allowing dialplan to do the same?

Back in ~2007 I wrote EXTERNAL() and pushed it up for review and got feedback that this might lead to deadlocks.  We've been using EXTERNAL() which is just about identical to the review here since that timeframe across many Asterisk versions and workloads and not once triggered a deadlock related to setting a variable on a channel we don't "own" either via Set(EXTERNAL(<channel>,<var>)=...) or AMI Setvar.

I've reviewed the code here and it is indeed in the exact same (safe) flow as action_setvar.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15893
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I7492645ae4307553d0f586d78e13a4f586231fdf
Gerrit-Change-Number: 15893
Gerrit-PatchSet: 7
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Murawski <markm at intellasoft.net>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Attention: N A <mail at interlinked.x10host.com>
Gerrit-Comment-Date: Thu, 10 Feb 2022 00:32:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220209/13050ec2/attachment.html>