pbx_variables.c: Misc fixes in variable substitution.

* Copy more than one character at a time when there is nothing to
substitute.

* Fix off by one error if a '}' or ']' is missing.

* Eliminated the requirement that the "used" parameter had to point to a
variable.  The current callers were always declaring a variable to meet
the requirement and discarding the value put into that variable.  Now it
can be NULL.

* In ast_str_substitute_variables_full() fixed using the bogus channel to
evaluate a function.  We were not using the bogus channel we just created
to help evaluate a subexpression.

Change-Id: Ia83d99f4f16abe47f329eb39b6ff2013ae7c9854

include/asterisk/pbx.h

@@ -1432,7 +1432,7 @@ void ast_str_substitute_variables_varshead(struct ast_str **buf, ssize_t maxlen,
  * \param c Channel variables from which to extract values, and channel to pass to any dialplan functions.
  * \param headp If no channel is specified, a channel list from which to extract variable values
  * \param templ Variable template to expand.
- * \param used Number of bytes read from the template.
+ * \param used Number of bytes read from the template.  (May be NULL)
  */
 void ast_str_substitute_variables_full(struct ast_str **buf, ssize_t maxlen, struct ast_channel *c, struct varshead *headp, const char *templ, size_t *used);
 /*! @} */

main/pbx_variables.c

@@ -396,51 +396,74 @@ const char *ast_str_retrieve_variable(struct ast_str **str, ssize_t maxlen, stru
 void ast_str_substitute_variables_full(struct ast_str **buf, ssize_t maxlen, struct ast_channel *c, struct varshead *headp, const char *templ, size_t *used)
 {
 	/* Substitutes variables into buf, based on string templ */
-	char *cp4 = NULL;
 	const char *whereweare;
-	int orig_size = 0;
+	struct ast_str *substr1 = ast_str_create(16);
-	int offset, offset2, isfunction;
+	struct ast_str *substr2 = NULL;
-	const char *nextvar, *nextexp, *nextthing;
+	struct ast_str *substr3 = ast_str_create(16);
-	const char *vars, *vare;
-	char *finalvars;
-	int pos, brackets, needsub, len;
-	struct ast_str *substr1 = ast_str_create(16), *substr2 = NULL, *substr3 = ast_str_create(16);
 
 	ast_str_reset(*buf);
+
+	if (!substr1 || !substr3) {
+		if (used) {
+			*used = ast_str_strlen(*buf);
+		}
+		ast_free(substr1);
+		ast_free(substr3);
+		return;
+	}
+
 	whereweare = templ;
 	while (!ast_strlen_zero(whereweare)) {
+		const char *nextvar = NULL;
+		const char *nextexp = NULL;
+		const char *nextthing;
+		const char *vars;
+		const char *vare;
+		char *finalvars;
+		int pos;
+		int brackets;
+		int needsub;
+		int len;
+
 		/* reset our buffer */
 		ast_str_reset(substr3);
 
-		/* Assume we're copying the whole remaining string */
+		/* Determine how much simply needs to be copied to the output buf. */
-		pos = strlen(whereweare);
-		nextvar = NULL;
-		nextexp = NULL;
 		nextthing = strchr(whereweare, '$');
 		if (nextthing) {
+			pos = nextthing - whereweare;
 			switch (nextthing[1]) {
 			case '{':
+				/* Variable substitution */
 				nextvar = nextthing;
-				pos = nextvar - whereweare;
 				break;
 			case '[':
+				/* Expression substitution */
 				nextexp = nextthing;
-				pos = nextexp - whereweare;
 				break;
 			default:
-				pos = 1;
+				/* '$' is not part of a substitution so include it too. */
+				++pos;
+				break;
 			}
+		} else {
+			/* We're copying the whole remaining string */
+			pos = strlen(whereweare);
 		}
 
 		if (pos) {
 			/* Copy that many bytes */
 			ast_str_append_substr(buf, maxlen, whereweare, pos);
 
-			templ += pos;
 			whereweare += pos;
 		}
 
 		if (nextvar) {
+			int offset;
+			int offset2;
+			int isfunction;
+			int res;
+
 			/* We have a variable.  Find the start and end, and determine
 			   if we are going to have to recursively call ourselves on the
 			   contents */
@@ -452,33 +475,42 @@ void ast_str_substitute_variables_full(struct ast_str **buf, ssize_t maxlen, str
 			while (brackets && *vare) {
 				if ((vare[0] == '$') && (vare[1] == '{')) {
 					needsub++;
+					brackets++;
+					vare++;
 				} else if (vare[0] == '{') {
 					brackets++;
 				} else if (vare[0] == '}') {
 					brackets--;
-				} else if ((vare[0] == '$') && (vare[1] == '['))
+				} else if ((vare[0] == '$') && (vare[1] == '[')) {
 					needsub++;
+					vare++;
+				}
 				vare++;
 			}
-			if (brackets)
+			len = vare - vars;
+			if (brackets) {
 				ast_log(LOG_WARNING, "Error in extension logic (missing '}')\n");
-			len = vare - vars - 1;
+			} else {
+				/* Don't count the closing '}' in the length. */
+				--len;
+			}
 
 			/* Skip totally over variable string */
-			whereweare += (len + 3);
+			whereweare = vare;
 
-			/* Store variable name (and truncate) */
+			/* Store variable name expression to lookup. */
 			ast_str_set_substr(&substr1, 0, vars, len);
 			ast_debug(5, "Evaluating '%s' (from '%s' len %d)\n", ast_str_buffer(substr1), vars, len);
 
 			/* Substitute if necessary */
 			if (needsub) {
-				size_t my_used;
-
 				if (!substr2) {
 					substr2 = ast_str_create(16);
+					if (!substr2) {
+						continue;
+					}
 				}
-				ast_str_substitute_variables_full(&substr2, 0, c, headp, ast_str_buffer(substr1), &my_used);
+				ast_str_substitute_variables_full(&substr2, 0, c, headp, ast_str_buffer(substr1), NULL);
 				finalvars = ast_str_buffer(substr2);
 			} else {
 				finalvars = ast_str_buffer(substr1);
@@ -488,28 +520,32 @@ void ast_str_substitute_variables_full(struct ast_str **buf, ssize_t maxlen, str
 			if (isfunction) {
 				/* Evaluate function */
 				if (c || !headp) {
-					cp4 = ast_func_read2(c, finalvars, &substr3, 0) ? NULL : ast_str_buffer(substr3);
+					res = ast_func_read2(c, finalvars, &substr3, 0);
 				} else {
 					struct varshead old;
-					struct ast_channel *bogus = ast_dummy_channel_alloc();
+					struct ast_channel *bogus;
+
+					bogus = ast_dummy_channel_alloc();
 					if (bogus) {
-						memcpy(&old, ast_channel_varshead(bogus), sizeof(old));
+						old = *ast_channel_varshead(bogus);
-						memcpy(ast_channel_varshead(bogus), headp, sizeof(*ast_channel_varshead(bogus)));
+						*ast_channel_varshead(bogus) = *headp;
-						cp4 = ast_func_read2(c, finalvars, &substr3, 0) ? NULL : ast_str_buffer(substr3);
+						res = ast_func_read2(bogus, finalvars, &substr3, 0);
 						/* Don't deallocate the varshead that was passed in */
-						memcpy(ast_channel_varshead(bogus), &old, sizeof(*ast_channel_varshead(bogus)));
+						*ast_channel_varshead(bogus) = old;
 						ast_channel_unref(bogus);
 					} else {
-						ast_log(LOG_ERROR, "Unable to allocate bogus channel for variable substitution.  Function results may be blank.\n");
+						ast_log(LOG_ERROR, "Unable to allocate bogus channel for function value substitution.\n");
+						res = -1;
 					}
 				}
-				ast_debug(2, "Function %s result is '%s'\n", finalvars, cp4 ? cp4 : "(null)");
+				ast_debug(2, "Function %s result is '%s'\n",
+					finalvars, res ? "" : ast_str_buffer(substr3));
 			} else {
 				/* Retrieve variable value */
 				ast_str_retrieve_variable(&substr3, 0, c, headp, finalvars);
-				cp4 = ast_str_buffer(substr3);
+				res = 0;
 			}
-			if (cp4) {
+			if (!res) {
 				ast_str_substring(substr3, offset, offset2);
 				ast_str_append(buf, maxlen, "%s", ast_str_buffer(substr3));
 			}
@@ -537,24 +573,29 @@ void ast_str_substitute_variables_full(struct ast_str **buf, ssize_t maxlen, str
 				}
 				vare++;
 			}
-			if (brackets)
+			len = vare - vars;
+			if (brackets) {
 				ast_log(LOG_WARNING, "Error in extension logic (missing ']')\n");
-			len = vare - vars - 1;
+			} else {
+				/* Don't count the closing ']' in the length. */
+				--len;
+			}
 
 			/* Skip totally over expression */
-			whereweare += (len + 3);
+			whereweare = vare;
 
-			/* Store variable name (and truncate) */
+			/* Store expression to evaluate. */
 			ast_str_set_substr(&substr1, 0, vars, len);
 
 			/* Substitute if necessary */
 			if (needsub) {
-				size_t my_used;
-
 				if (!substr2) {
 					substr2 = ast_str_create(16);
+					if (!substr2) {
+						continue;
+					}
 				}
-				ast_str_substitute_variables_full(&substr2, 0, c, headp, ast_str_buffer(substr1), &my_used);
+				ast_str_substitute_variables_full(&substr2, 0, c, headp, ast_str_buffer(substr1), NULL);
 				finalvars = ast_str_buffer(substr2);
 			} else {
 				finalvars = ast_str_buffer(substr1);
@@ -566,7 +607,9 @@ void ast_str_substitute_variables_full(struct ast_str **buf, ssize_t maxlen, str
 			ast_str_append(buf, maxlen, "%s", ast_str_buffer(substr3));
 		}
 	}
-	*used = ast_str_strlen(*buf) - orig_size;
+	if (used) {
+		*used = ast_str_strlen(*buf);
+	}
 	ast_free(substr1);
 	ast_free(substr2);
 	ast_free(substr3);
@@ -574,49 +617,58 @@ void ast_str_substitute_variables_full(struct ast_str **buf, ssize_t maxlen, str
 
 void ast_str_substitute_variables(struct ast_str **buf, ssize_t maxlen, struct ast_channel *chan, const char *templ)
 {
-	size_t used;
+	ast_str_substitute_variables_full(buf, maxlen, chan, NULL, templ, NULL);
-	ast_str_substitute_variables_full(buf, maxlen, chan, NULL, templ, &used);
 }
 
 void ast_str_substitute_variables_varshead(struct ast_str **buf, ssize_t maxlen, struct varshead *headp, const char *templ)
 {
-	size_t used;
+	ast_str_substitute_variables_full(buf, maxlen, NULL, headp, templ, NULL);
-	ast_str_substitute_variables_full(buf, maxlen, NULL, headp, templ, &used);
 }
 
 void pbx_substitute_variables_helper_full(struct ast_channel *c, struct varshead *headp, const char *cp1, char *cp2, int count, size_t *used)
 {
 	/* Substitutes variables into cp2, based on string cp1, cp2 NO LONGER NEEDS TO BE ZEROED OUT!!!!  */
-	char *cp4 = NULL;
+	const char *whereweare;
-	const char *whereweare, *orig_cp2 = cp2;
+	const char *orig_cp2 = cp2;
-	int length, offset, offset2, isfunction;
 	char *workspace = NULL;
-	char *ltmp = NULL, *var = NULL;
+	char *ltmp = NULL;
-	char *nextvar, *nextexp, *nextthing;
+	char *var = NULL;
-	char *vars, *vare;
-	int pos, brackets, needsub, len;
 
 	*cp2 = 0; /* just in case nothing ends up there */
 	whereweare = cp1;
 	while (!ast_strlen_zero(whereweare) && count) {
-		/* Assume we're copying the whole remaining string */
+		char *nextvar = NULL;
-		pos = strlen(whereweare);
+		char *nextexp = NULL;
-		nextvar = NULL;
+		char *nextthing;
-		nextexp = NULL;
+		char *vars;
+		char *vare;
+		int length;
+		int pos;
+		int brackets;
+		int needsub;
+		int len;
+
+		/* Determine how much simply needs to be copied to the output buf. */
 		nextthing = strchr(whereweare, '$');
 		if (nextthing) {
+			pos = nextthing - whereweare;
 			switch (nextthing[1]) {
 			case '{':
+				/* Variable substitution */
 				nextvar = nextthing;
-				pos = nextvar - whereweare;
 				break;
 			case '[':
+				/* Expression substitution */
 				nextexp = nextthing;
-				pos = nextexp - whereweare;
 				break;
 			default:
-				pos = 1;
+				/* '$' is not part of a substitution so include it too. */
+				++pos;
+				break;
 			}
+		} else {
+			/* We're copying the whole remaining string */
+			pos = strlen(whereweare);
 		}
 
 		if (pos) {
@@ -634,6 +686,11 @@ void pbx_substitute_variables_helper_full(struct ast_channel *c, struct varshead
 		}
 
 		if (nextvar) {
+			int offset;
+			int offset2;
+			int isfunction;
+			char *cp4;
+
 			/* We have a variable.  Find the start and end, and determine
 			   if we are going to have to recursively call ourselves on the
 			   contents */
@@ -645,35 +702,41 @@ void pbx_substitute_variables_helper_full(struct ast_channel *c, struct varshead
 			while (brackets && *vare) {
 				if ((vare[0] == '$') && (vare[1] == '{')) {
 					needsub++;
+					brackets++;
+					vare++;
 				} else if (vare[0] == '{') {
 					brackets++;
 				} else if (vare[0] == '}') {
 					brackets--;
-				} else if ((vare[0] == '$') && (vare[1] == '['))
+				} else if ((vare[0] == '$') && (vare[1] == '[')) {
 					needsub++;
+					vare++;
+				}
 				vare++;
 			}
-			if (brackets)
+			len = vare - vars;
+			if (brackets) {
 				ast_log(LOG_WARNING, "Error in extension logic (missing '}')\n");
-			len = vare - vars - 1;
+			} else {
+				/* Don't count the closing '}' in the length. */
+				--len;
+			}
 
 			/* Skip totally over variable string */
-			whereweare += (len + 3);
+			whereweare = vare;
 
 			if (!var)
 				var = ast_alloca(VAR_BUF_SIZE);
 
-			/* Store variable name (and truncate) */
+			/* Store variable name expression to lookup (and truncate). */
 			ast_copy_string(var, vars, len + 1);
 
 			/* Substitute if necessary */
 			if (needsub) {
-				size_t my_used;
-
 				if (!ltmp) {
 					ltmp = ast_alloca(VAR_BUF_SIZE);
 				}
-				pbx_substitute_variables_helper_full(c, headp, var, ltmp, VAR_BUF_SIZE - 1, &my_used);
+				pbx_substitute_variables_helper_full(c, headp, var, ltmp, VAR_BUF_SIZE - 1, NULL);
 				vars = ltmp;
 			} else {
 				vars = var;
@@ -691,16 +754,19 @@ void pbx_substitute_variables_helper_full(struct ast_channel *c, struct varshead
 					cp4 = ast_func_read(c, vars, workspace, VAR_BUF_SIZE) ? NULL : workspace;
 				else {
 					struct varshead old;
-					struct ast_channel *c = ast_dummy_channel_alloc();
+					struct ast_channel *bogus;
-					if (c) {
+
-						memcpy(&old, ast_channel_varshead(c), sizeof(old));
+					bogus = ast_dummy_channel_alloc();
-						memcpy(ast_channel_varshead(c), headp, sizeof(*ast_channel_varshead(c)));
+					if (bogus) {
-						cp4 = ast_func_read(c, vars, workspace, VAR_BUF_SIZE) ? NULL : workspace;
+						old = *ast_channel_varshead(bogus);
+						*ast_channel_varshead(bogus) = *headp;
+						cp4 = ast_func_read(bogus, vars, workspace, VAR_BUF_SIZE) ? NULL : workspace;
 						/* Don't deallocate the varshead that was passed in */
-						memcpy(ast_channel_varshead(c), &old, sizeof(*ast_channel_varshead(c)));
+						*ast_channel_varshead(bogus) = old;
-						c = ast_channel_unref(c);
+						ast_channel_unref(bogus);
 					} else {
-						ast_log(LOG_ERROR, "Unable to allocate bogus channel for variable substitution.  Function results may be blank.\n");
+						ast_log(LOG_ERROR, "Unable to allocate bogus channel for function value substitution.\n");
+						cp4 = NULL;
 					}
 				}
 				ast_debug(2, "Function %s result is '%s'\n", vars, cp4 ? cp4 : "(null)");
@@ -743,34 +809,35 @@ void pbx_substitute_variables_helper_full(struct ast_channel *c, struct varshead
 				}
 				vare++;
 			}
-			if (brackets)
+			len = vare - vars;
+			if (brackets) {
 				ast_log(LOG_WARNING, "Error in extension logic (missing ']')\n");
-			len = vare - vars - 1;
+			} else {
+				/* Don't count the closing ']' in the length. */
+				--len;
+			}
 
 			/* Skip totally over expression */
-			whereweare += (len + 3);
+			whereweare = vare;
 
 			if (!var)
 				var = ast_alloca(VAR_BUF_SIZE);
 
-			/* Store variable name (and truncate) */
+			/* Store expression to evaluate (and truncate). */
 			ast_copy_string(var, vars, len + 1);
 
 			/* Substitute if necessary */
 			if (needsub) {
-				size_t my_used;
-
 				if (!ltmp) {
 					ltmp = ast_alloca(VAR_BUF_SIZE);
 				}
-				pbx_substitute_variables_helper_full(c, headp, var, ltmp, VAR_BUF_SIZE - 1, &my_used);
+				pbx_substitute_variables_helper_full(c, headp, var, ltmp, VAR_BUF_SIZE - 1, NULL);
 				vars = ltmp;
 			} else {
 				vars = var;
 			}
 
 			length = ast_expr(vars, cp2, count, c);
-
 			if (length) {
 				ast_debug(1, "Expression result is '%s'\n", cp2);
 				count -= length;
@@ -779,19 +846,19 @@ void pbx_substitute_variables_helper_full(struct ast_channel *c, struct varshead
 			}
 		}
 	}
-	*used = cp2 - orig_cp2;
+	if (used) {
+		*used = cp2 - orig_cp2;
+	}
 }
 
 void pbx_substitute_variables_helper(struct ast_channel *c, const char *cp1, char *cp2, int count)
 {
-	size_t used;
+	pbx_substitute_variables_helper_full(c, (c) ? ast_channel_varshead(c) : NULL, cp1, cp2, count, NULL);
-	pbx_substitute_variables_helper_full(c, (c) ? ast_channel_varshead(c) : NULL, cp1, cp2, count, &used);
 }
 
 void pbx_substitute_variables_varshead(struct varshead *headp, const char *cp1, char *cp2, int count)
 {
-	size_t used;
+	pbx_substitute_variables_helper_full(NULL, headp, cp1, cp2, count, NULL);
-	pbx_substitute_variables_helper_full(NULL, headp, cp1, cp2, count, &used);
 }
 
 /*! \brief CLI support for listing global variables in a parseable way */