<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6310">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
George Joseph: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">app_record: Resolve some absolute vs. relative filename bugs<br><br>If the Record() application is called with a relative filename that<br>includes directories, we were not properly creating the intermediate<br>directories and Record() would fail.<br><br>Secondarily, updated the documentation for RECORDED_FILE to mention<br>that it does not include a filename extension.<br><br>Finally, rewrote the '%d' functionality to be a bit more straight<br>forward and less noisy.<br><br>ASTERISK-16777 #close<br>Reported by: klaus3000<br><br>Change-Id: Ibc2640cba3a8c7f17d97b02f76b7608b1e7ffde2<br>---<br>M apps/app_record.c<br>1 file changed, 68 insertions(+), 45 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/apps/app_record.c b/apps/app_record.c<br>index 0b85ff8..8c3a577 100644<br>--- a/apps/app_record.c<br>+++ b/apps/app_record.c<br>@@ -38,6 +38,7 @@<br> #include "asterisk/channel.h"<br> #include "asterisk/dsp.h" /* use dsp routines for silence detection */<br> #include "asterisk/format_cache.h"<br>+#include "asterisk/paths.h"<br> <br> /*** DOCUMENTATION<br> <application name="Record" language="en_US"><br>@@ -102,7 +103,7 @@<br> If the user hangs up during a recording, all data will be lost and the application will terminate.</para><br> <variablelist><br> <variable name="RECORDED_FILE"><br>- <para>Will be set to the final filename of the recording.</para><br>+ <para>Will be set to the final filename of the recording, without an extension.</para><br> </variable><br> <variable name="RECORD_STATUS"><br> <para>This is the final status of the command</para><br>@@ -131,10 +132,9 @@<br> OPTION_STAR_TERMINATE = (1 << 4),<br> OPTION_IGNORE_TERMINATE = (1 << 5),<br> OPTION_KEEP = (1 << 6),<br>- FLAG_HAS_PERCENT = (1 << 7),<br>- OPTION_ANY_TERMINATE = (1 << 8),<br>- OPTION_OPERATOR_EXIT = (1 << 9),<br>- OPTION_NO_TRUNCATE = (1 << 10),<br>+ OPTION_ANY_TERMINATE = (1 << 7),<br>+ OPTION_OPERATOR_EXIT = (1 << 8),<br>+ OPTION_NO_TRUNCATE = (1 << 9),<br> };<br> <br> AST_APP_OPTIONS(app_opts,{<br>@@ -180,14 +180,47 @@<br> return 0;<br> }<br> <br>+static int create_destination_directory(const char *path)<br>+{<br>+ int res;<br>+ char directory[PATH_MAX], *file_sep;<br>+<br>+ if (!(file_sep = strrchr(path, '/'))) {<br>+ /* No directory to create */<br>+ return 0;<br>+ }<br>+<br>+ /* Overwrite temporarily */<br>+ *file_sep = '\0';<br>+<br>+ /* Absolute path? */<br>+ if (path[0] == '/') {<br>+ res = ast_mkdir(path, 0777);<br>+ *file_sep = '/';<br>+ return res;<br>+ }<br>+<br>+ /* Relative path */<br>+ res = snprintf(directory, sizeof(directory), "%s/sounds/%s",<br>+ ast_config_AST_DATA_DIR, path);<br>+<br>+ *file_sep = '/';<br>+<br>+ if (res >= sizeof(directory)) {<br>+ /* We truncated, so we fail */<br>+ return -1;<br>+ }<br>+<br>+ return ast_mkdir(directory, 0777);<br>+}<br>+<br> static int record_exec(struct ast_channel *chan, const char *data)<br> {<br> int res = 0;<br>- int count = 0;<br> char *ext = NULL, *opts[0];<br>- char *parse, *dir, *file;<br>+ char *parse;<br> int i = 0;<br>- char tmp[256];<br>+ char tmp[PATH_MAX];<br> <br> struct ast_filestream *s = NULL;<br> struct ast_frame *f = NULL;<br>@@ -227,8 +260,6 @@<br> ast_app_parse_options(app_opts, &flags, opts, args.options);<br> <br> if (!ast_strlen_zero(args.filename)) {<br>- if (strstr(args.filename, "%d"))<br>- ast_set_flag(&flags, FLAG_HAS_PERCENT);<br> ext = strrchr(args.filename, '.'); /* to support filename with a . in the filename, not format */<br> if (!ext)<br> ext = strchr(args.filename, ':');<br>@@ -266,38 +297,31 @@<br> if (ast_test_flag(&flags, OPTION_IGNORE_TERMINATE))<br> terminator = '\0';<br> <br>- /* done parsing */<br>+ /*<br>+ If a '%d' is specified as part of the filename, we replace that token with<br>+ sequentially incrementing numbers until we find a unique filename.<br>+ */<br>+ if (strchr(args.filename, '%')) {<br>+ size_t src, dst, count = 0;<br>+ size_t src_len = strlen(args.filename);<br>+ size_t dst_len = sizeof(tmp) - 1;<br> <br>- /* these are to allow the use of the %d in the config file for a wild card of sort to<br>- create a new file with the inputed name scheme */<br>- if (ast_test_flag(&flags, FLAG_HAS_PERCENT)) {<br>- AST_DECLARE_APP_ARGS(fname,<br>- AST_APP_ARG(piece)[100];<br>- );<br>- char *tmp2 = ast_strdupa(args.filename);<br>- char countstring[15];<br>- int idx;<br>-<br>- /* Separate each piece out by the format specifier */<br>- AST_NONSTANDARD_APP_ARGS(fname, tmp2, '%');<br> do {<br>- int tmplen;<br>- /* First piece has no leading percent, so it's copied verbatim */<br>- ast_copy_string(tmp, fname.piece[0], sizeof(tmp));<br>- tmplen = strlen(tmp);<br>- for (idx = 1; idx < fname.argc; idx++) {<br>- if (fname.piece[idx][0] == 'd') {<br>- /* Substitute the count */<br>- snprintf(countstring, sizeof(countstring), "%d", count);<br>- ast_copy_string(tmp + tmplen, countstring, sizeof(tmp) - tmplen);<br>- tmplen += strlen(countstring);<br>- } else if (tmplen + 2 < sizeof(tmp)) {<br>- /* Unknown format specifier - just copy it verbatim */<br>- tmp[tmplen++] = '%';<br>- tmp[tmplen++] = fname.piece[idx][0];<br>+ for (src = 0, dst = 0; src < src_len && dst < dst_len; src++) {<br>+ if (!strncmp(&args.filename[src], "%d", 2)) {<br>+ int s = snprintf(&tmp[dst], PATH_MAX - dst, "%zu", count);<br>+ if (s >= PATH_MAX - dst) {<br>+ /* We truncated, so we need to bail */<br>+ ast_log(LOG_WARNING, "Failed to create unique filename from template: %s\n", args.filename);<br>+ pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "ERROR");<br>+ return -1;<br>+ }<br>+ dst += s;<br>+ src++;<br>+ } else {<br>+ tmp[dst] = args.filename[src];<br>+ tmp[++dst] = '\0';<br> }<br>- /* Copy the remaining portion of the piece */<br>- ast_copy_string(tmp + tmplen, &(fname.piece[idx][1]), sizeof(tmp) - tmplen);<br> }<br> count++;<br> } while (ast_fileexists(tmp, ext, ast_channel_language(chan)) > 0);<br>@@ -305,7 +329,6 @@<br> ast_copy_string(tmp, args.filename, sizeof(tmp));<br> <br> pbx_builtin_setvar_helper(chan, "RECORDED_FILE", tmp);<br>- /* end of routine mentioned */<br> <br> if (ast_channel_state(chan) != AST_STATE_UP) {<br> if (ast_test_flag(&flags, OPTION_SKIP)) {<br>@@ -354,11 +377,11 @@<br> ast_dsp_set_threshold(sildet, ast_dsp_get_threshold_from_settings(THRESHOLD_SILENCE));<br> } <br> <br>- /* Create the directory if it does not exist. */<br>- dir = ast_strdupa(tmp);<br>- if ((file = strrchr(dir, '/')))<br>- *file++ = '\0';<br>- ast_mkdir (dir, 0777);<br>+ if (create_destination_directory(tmp)) {<br>+ ast_log(LOG_WARNING, "Could not create directory for file %s\n", args.filename);<br>+ pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "ERROR");<br>+ goto out;<br>+ }<br> <br> ioflags = ast_test_flag(&flags, OPTION_APPEND) ? O_CREAT|O_APPEND|O_WRONLY : O_CREAT|O_TRUNC|O_WRONLY;<br> s = ast_writefile(tmp, ext, NULL, ioflags, 0, AST_FILE_MODE);<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6310">change 6310</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/6310"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ibc2640cba3a8c7f17d97b02f76b7608b1e7ffde2 </div>
<div style="display:none"> Gerrit-Change-Number: 6310 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>