[Asterisk-code-review] app record: Resolve some absolute vs. relative filename bugs (asterisk[15])

Joshua Colp asteriskteam at digium.com
Tue Aug 29 05:48:10 CDT 2017


Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/6310 )

Change subject: app_record: Resolve some absolute vs. relative filename bugs
......................................................................

app_record: Resolve some absolute vs. relative filename bugs

If the Record() application is called with a relative filename that
includes directories, we were not properly creating the intermediate
directories and Record() would fail.

Secondarily, updated the documentation for RECORDED_FILE to mention
that it does not include a filename extension.

Finally, rewrote the '%d' functionality to be a bit more straight
forward and less noisy.

ASTERISK-16777 #close
Reported by: klaus3000

Change-Id: Ibc2640cba3a8c7f17d97b02f76b7608b1e7ffde2
---
M apps/app_record.c
1 file changed, 68 insertions(+), 45 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
  George Joseph: Looks good to me, approved



diff --git a/apps/app_record.c b/apps/app_record.c
index 0b85ff8..8c3a577 100644
--- a/apps/app_record.c
+++ b/apps/app_record.c
@@ -38,6 +38,7 @@
 #include "asterisk/channel.h"
 #include "asterisk/dsp.h"	/* use dsp routines for silence detection */
 #include "asterisk/format_cache.h"
+#include "asterisk/paths.h"
 
 /*** DOCUMENTATION
 	<application name="Record" language="en_US">
@@ -102,7 +103,7 @@
 			If the user hangs up during a recording, all data will be lost and the application will terminate.</para>
 			<variablelist>
 				<variable name="RECORDED_FILE">
-					<para>Will be set to the final filename of the recording.</para>
+					<para>Will be set to the final filename of the recording, without an extension.</para>
 				</variable>
 				<variable name="RECORD_STATUS">
 					<para>This is the final status of the command</para>
@@ -131,10 +132,9 @@
 	OPTION_STAR_TERMINATE = (1 << 4),
 	OPTION_IGNORE_TERMINATE = (1 << 5),
 	OPTION_KEEP = (1 << 6),
-	FLAG_HAS_PERCENT = (1 << 7),
-	OPTION_ANY_TERMINATE = (1 << 8),
-	OPTION_OPERATOR_EXIT = (1 << 9),
-	OPTION_NO_TRUNCATE = (1 << 10),
+	OPTION_ANY_TERMINATE = (1 << 7),
+	OPTION_OPERATOR_EXIT = (1 << 8),
+	OPTION_NO_TRUNCATE = (1 << 9),
 };
 
 AST_APP_OPTIONS(app_opts,{
@@ -180,14 +180,47 @@
 	return 0;
 }
 
+static int create_destination_directory(const char *path)
+{
+	int res;
+	char directory[PATH_MAX], *file_sep;
+
+	if (!(file_sep = strrchr(path, '/'))) {
+		/* No directory to create */
+		return 0;
+	}
+
+	/* Overwrite temporarily */
+	*file_sep = '\0';
+
+	/* Absolute path? */
+	if (path[0] == '/') {
+		res = ast_mkdir(path, 0777);
+		*file_sep = '/';
+		return res;
+	}
+
+	/* Relative path */
+	res = snprintf(directory, sizeof(directory), "%s/sounds/%s",
+				   ast_config_AST_DATA_DIR, path);
+
+	*file_sep = '/';
+
+	if (res >= sizeof(directory)) {
+		/* We truncated, so we fail */
+		return -1;
+	}
+
+	return ast_mkdir(directory, 0777);
+}
+
 static int record_exec(struct ast_channel *chan, const char *data)
 {
 	int res = 0;
-	int count = 0;
 	char *ext = NULL, *opts[0];
-	char *parse, *dir, *file;
+	char *parse;
 	int i = 0;
-	char tmp[256];
+	char tmp[PATH_MAX];
 
 	struct ast_filestream *s = NULL;
 	struct ast_frame *f = NULL;
@@ -227,8 +260,6 @@
 		ast_app_parse_options(app_opts, &flags, opts, args.options);
 
 	if (!ast_strlen_zero(args.filename)) {
-		if (strstr(args.filename, "%d"))
-			ast_set_flag(&flags, FLAG_HAS_PERCENT);
 		ext = strrchr(args.filename, '.'); /* to support filename with a . in the filename, not format */
 		if (!ext)
 			ext = strchr(args.filename, ':');
@@ -266,38 +297,31 @@
 	if (ast_test_flag(&flags, OPTION_IGNORE_TERMINATE))
 		terminator = '\0';
 
-	/* done parsing */
+	/*
+	  If a '%d' is specified as part of the filename, we replace that token with
+	  sequentially incrementing numbers until we find a unique filename.
+	*/
+	if (strchr(args.filename, '%')) {
+		size_t src, dst, count = 0;
+		size_t src_len = strlen(args.filename);
+		size_t dst_len = sizeof(tmp) - 1;
 
-	/* these are to allow the use of the %d in the config file for a wild card of sort to
-	  create a new file with the inputed name scheme */
-	if (ast_test_flag(&flags, FLAG_HAS_PERCENT)) {
-		AST_DECLARE_APP_ARGS(fname,
-			AST_APP_ARG(piece)[100];
-		);
-		char *tmp2 = ast_strdupa(args.filename);
-		char countstring[15];
-		int idx;
-
-		/* Separate each piece out by the format specifier */
-		AST_NONSTANDARD_APP_ARGS(fname, tmp2, '%');
 		do {
-			int tmplen;
-			/* First piece has no leading percent, so it's copied verbatim */
-			ast_copy_string(tmp, fname.piece[0], sizeof(tmp));
-			tmplen = strlen(tmp);
-			for (idx = 1; idx < fname.argc; idx++) {
-				if (fname.piece[idx][0] == 'd') {
-					/* Substitute the count */
-					snprintf(countstring, sizeof(countstring), "%d", count);
-					ast_copy_string(tmp + tmplen, countstring, sizeof(tmp) - tmplen);
-					tmplen += strlen(countstring);
-				} else if (tmplen + 2 < sizeof(tmp)) {
-					/* Unknown format specifier - just copy it verbatim */
-					tmp[tmplen++] = '%';
-					tmp[tmplen++] = fname.piece[idx][0];
+			for (src = 0, dst = 0; src < src_len && dst < dst_len; src++) {
+				if (!strncmp(&args.filename[src], "%d", 2)) {
+					int s = snprintf(&tmp[dst], PATH_MAX - dst, "%zu", count);
+					if (s >= PATH_MAX - dst) {
+						/* We truncated, so we need to bail */
+						ast_log(LOG_WARNING, "Failed to create unique filename from template: %s\n", args.filename);
+						pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "ERROR");
+						return -1;
+					}
+					dst += s;
+					src++;
+				} else {
+					tmp[dst] = args.filename[src];
+					tmp[++dst] = '\0';
 				}
-				/* Copy the remaining portion of the piece */
-				ast_copy_string(tmp + tmplen, &(fname.piece[idx][1]), sizeof(tmp) - tmplen);
 			}
 			count++;
 		} while (ast_fileexists(tmp, ext, ast_channel_language(chan)) > 0);
@@ -305,7 +329,6 @@
 		ast_copy_string(tmp, args.filename, sizeof(tmp));
 
 	pbx_builtin_setvar_helper(chan, "RECORDED_FILE", tmp);
-	/* end of routine mentioned */
 
 	if (ast_channel_state(chan) != AST_STATE_UP) {
 		if (ast_test_flag(&flags, OPTION_SKIP)) {
@@ -354,11 +377,11 @@
 		ast_dsp_set_threshold(sildet, ast_dsp_get_threshold_from_settings(THRESHOLD_SILENCE));
 	} 
 
-	/* Create the directory if it does not exist. */
-	dir = ast_strdupa(tmp);
-	if ((file = strrchr(dir, '/')))
-		*file++ = '\0';
-	ast_mkdir (dir, 0777);
+	if (create_destination_directory(tmp)) {
+		ast_log(LOG_WARNING, "Could not create directory for file %s\n", args.filename);
+		pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "ERROR");
+		goto out;
+	}
 
 	ioflags = ast_test_flag(&flags, OPTION_APPEND) ? O_CREAT|O_APPEND|O_WRONLY : O_CREAT|O_TRUNC|O_WRONLY;
 	s = ast_writefile(tmp, ext, NULL, ioflags, 0, AST_FILE_MODE);

-- 
To view, visit https://gerrit.asterisk.org/6310
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibc2640cba3a8c7f17d97b02f76b7608b1e7ffde2
Gerrit-Change-Number: 6310
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170829/fffbbbca/attachment.html>


More information about the asterisk-code-review mailing list