[Asterisk-code-review] func_odbc: Introduce minargs config and expose ARGC in addition to A... (asterisk[18])

Jaco Kroon asteriskteam at digium.com
Tue Feb 23 05:03:42 CST 2021


Jaco Kroon has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/15506 )


Change subject: func_odbc:  Introduce minargs config and expose ARGC in addition to ARGn.
......................................................................

func_odbc:  Introduce minargs config and expose ARGC in addition to ARGn.

minargs enables enforcing of minimum count of arguments to pass to
func_odbc, so if you're unconditionally using ARG1 through ARG4 then
this should be set to 4.  func_odbc will generate an error in this case,
so for example

[FOO]
minargs = 4

and ODBC_FOO(a,b,c) in dialplan will now error out instead of using a
potentially leaked ARG4 from Gosub().

ARGC is needed if you're using optional argument, to verify whether or
not an argument has been passed, else it's possible to use a leaked ARGn
from Gosub (app_stack).  So now you can safely do
${IF($[${ARGC}>3]?${ARGV}:default value)} kind of thing.

Change-Id: I6ca0b137d90b03f6aa9c496991f6cbf1518f6c24
Signed-off-by: Jaco Kroon <jaco at uls.co.za>
---
M configs/samples/func_odbc.conf.sample
A doc/CHANGES-staging/func_odbc_ARGC_minargs.txt
M funcs/func_odbc.c
3 files changed, 60 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/06/15506/1

diff --git a/configs/samples/func_odbc.conf.sample b/configs/samples/func_odbc.conf.sample
index c467f7e..b825974 100644
--- a/configs/samples/func_odbc.conf.sample
+++ b/configs/samples/func_odbc.conf.sample
@@ -23,6 +23,10 @@
 ; For substitution, you have ${ARG1}, ${ARG2} ... ${ARGn}
 ; for the arguments to each SQL statement.
 ;
+; Additionally you can use ${ARGC} to determine the number of arguments that
+; was actually passed (or risk using leaked ARGn variables from the channel).
+; Also reference the minargs configuration option.
+;
 ; In addition, for write statements, you have ${VAL1}, ${VAL2} ... ${VALn}
 ; parsed, just like arguments, for the values.  In addition, if you want the
 ; whole value, never mind the parsing, you can get that with ${VALUE}.
@@ -87,6 +91,13 @@
 ;              These additional rows can be returned by using the name of the
 ;              function which was called to retrieve the first row as an
 ;              argument to ODBC_FETCH().
+; minargs      The minimum number of ARGUMENTS that has to be passed to the
+;              function.  If fewer arguments than this is passed, then the call
+;              will fail.  It is important to note that unlike Gosub() and friends,
+;              func_odbc will not mask out ARGn variables that it's not actively
+;              using, as such, without this, it's entirely possible to use say
+;              ARG2 from the Gosub() inside func_odbc when the intent was to
+;              use an argument passed to func_odbc, but it simply was never passed.
 
 
 ; ODBC_SQL - Allow an SQL statement to be built entirely in the dialplan
diff --git a/doc/CHANGES-staging/func_odbc_ARGC_minargs.txt b/doc/CHANGES-staging/func_odbc_ARGC_minargs.txt
new file mode 100644
index 0000000..0984b50
--- /dev/null
+++ b/doc/CHANGES-staging/func_odbc_ARGC_minargs.txt
@@ -0,0 +1,20 @@
+Subject: func_odbc
+
+Introduce an ARGC variable for func_odbc functions, along with a minargs
+per-function configuration option.
+
+minargs enables enforcing of minimum count of arguments to pass to
+func_odbc, so if you're unconditionally using ARG1 through ARG4 then
+this should be set to 4.  func_odbc will generate an error in this case,
+so for example
+
+[FOO]
+minargs = 4
+
+and ODBC_FOO(a,b,c) in dialplan will now error out instead of using a
+potentially leaked ARG4 from Gosub().
+
+ARGC is needed if you're using optional argument, to verify whether or
+not an argument has been passed, else it's possible to use a leaked ARGn
+from Gosub (app_stack).  So now you can safely do
+${IF($[${ARGC}>3]?${ARGV}:default value)} kind of thing.
diff --git a/funcs/func_odbc.c b/funcs/func_odbc.c
index 5cc0faa..9d6d0fc 100644
--- a/funcs/func_odbc.c
+++ b/funcs/func_odbc.c
@@ -120,6 +120,7 @@
 	char *sql_insert;
 	unsigned int flags;
 	int rowlimit;
+	int minargs;
 	struct ast_custom_function *acf;
 };
 
@@ -545,6 +546,14 @@
 		return -1;
 	}
 
+	AST_STANDARD_APP_ARGS(args, s);
+	if (args.argc < query->minargs) {
+		ast_log(LOG_ERROR, "%d arguments supplied to '%s' requiring minimum %d\n",
+				args.argc, cmd, query->minargs);
+		AST_RWLIST_UNLOCK(&queries);
+		return -1;
+	}
+
 	if (!chan) {
 		if (!(chan = ast_dummy_channel_alloc())) {
 			AST_RWLIST_UNLOCK(&queries);
@@ -578,7 +587,8 @@
 		return -1;
 	}
 
-	AST_STANDARD_APP_ARGS(args, s);
+	snprintf(varname, sizeof(varname), "%u", args.argc);
+	pbx_builtin_pushvar_helper(chan, "ARGC", varname);
 	for (i = 0; i < args.argc; i++) {
 		snprintf(varname, sizeof(varname), "ARG%d", i + 1);
 		pbx_builtin_pushvar_helper(chan, varname, args.field[i]);
@@ -603,6 +613,8 @@
 		chan = ast_channel_unref(chan);
 	} else {
 		/* Restore prior values */
+		pbx_builtin_setvar_helper(chan, "ARGC", NULL);
+
 		for (i = 0; i < args.argc; i++) {
 			snprintf(varname, sizeof(varname), "ARG%d", i + 1);
 			pbx_builtin_setvar_helper(chan, varname, NULL);
@@ -756,6 +768,14 @@
 		return -1;
 	}
 
+	AST_STANDARD_APP_ARGS(args, s);
+	if (args.argc < query->minargs) {
+		ast_log(LOG_ERROR, "%d arguments supplied to '%s' requiring minimum %d\n",
+				args.argc, cmd, query->minargs);
+		AST_RWLIST_UNLOCK(&queries);
+		return -1;
+	}
+
 	if (!chan) {
 		if (!(chan = ast_dummy_channel_alloc())) {
 			AST_RWLIST_UNLOCK(&queries);
@@ -768,7 +788,8 @@
 		ast_autoservice_start(chan);
 	}
 
-	AST_STANDARD_APP_ARGS(args, s);
+	snprintf(varname, sizeof(varname), "%u", args.argc);
+	pbx_builtin_pushvar_helper(chan, "ARGC", varname);
 	for (x = 0; x < args.argc; x++) {
 		snprintf(varname, sizeof(varname), "ARG%d", x + 1);
 		pbx_builtin_pushvar_helper(chan, varname, args.field[x]);
@@ -780,6 +801,8 @@
 		chan = ast_channel_unref(chan);
 	} else {
 		/* Restore prior values */
+		pbx_builtin_setvar_helper(chan, "ARGC", NULL);
+
 		for (x = 0; x < args.argc; x++) {
 			snprintf(varname, sizeof(varname), "ARG%d", x + 1);
 			pbx_builtin_setvar_helper(chan, varname, NULL);
@@ -1290,6 +1313,10 @@
 			sscanf(tmp, "%30d", &((*query)->rowlimit));
 	}
 
+	if ((tmp = ast_variable_retrieve(cfg, catg, "minargs"))) {
+		sscanf(tmp, "%30d", &((*query)->minargs));
+	}
+
 	(*query)->acf = ast_calloc(1, sizeof(struct ast_custom_function));
 	if (!(*query)->acf) {
 		free_acf_query(*query);

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

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: I6ca0b137d90b03f6aa9c496991f6cbf1518f6c24
Gerrit-Change-Number: 15506
Gerrit-PatchSet: 1
Gerrit-Owner: Jaco Kroon <jaco at uls.co.za>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210223/9f253abe/attachment-0001.html>


More information about the asterisk-code-review mailing list