[Asterisk-code-review] media cache: make sure generated filenames cannot conflict (asterisk[master])

Nathan Bruning asteriskteam at digium.com
Wed Feb 8 05:56:17 CST 2023


Nathan Bruning has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/19892 )


Change subject: media cache: make sure generated filenames cannot conflict
......................................................................

media cache: make sure generated filenames cannot conflict

In rare cases, two entries of the media cache would have the same
base file name, only with different extensions. During media lookup,
the wrong file can be played, depending on the channel format.

This patch avoids the 'lookup file by extension' for files that
originate from the media cache.

ASTERISK-30408 #close

Change-Id: I89ad019b4344a8f2f09d388740f78751a8e6673b
---
M main/file.c
1 file changed, 129 insertions(+), 42 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/92/19892/1

diff --git a/main/file.c b/main/file.c
index d7c7543..871dedb 100644
--- a/main/file.c
+++ b/main/file.c
@@ -535,6 +535,71 @@
 	ACTION_COPY	/* copy file. return 0 on success, -1 on error */
 };
 
+int stream_file(const char *fn, struct ast_channel *chan, struct ast_format_def *f)
+{
+	FILE *bfile;
+	struct ast_filestream *s;
+
+	if (ast_format_cmp(ast_channel_writeformat(chan), f->format) == AST_FORMAT_CMP_NOT_EQUAL) {
+		return 0;       /* not a supported format */
+	}
+	if ( (bfile = fopen(fn, "r")) == NULL) {
+		return 0;       /* cannot open file */
+	}
+	s = get_filestream(f, bfile);
+	if (!s) {
+		fclose(bfile);
+		return 0; /* cannot allocate descriptor */
+	}
+	if (open_wrapper(s)) {
+		ast_closestream(s);
+		return 0;       /* cannot run open on file */
+	}
+	/* ok this is good for OPEN */
+	s->lasttimeout = -1;
+	s->fmt = f;
+	s->trans = NULL;
+	s->filename = NULL;
+	if (ast_format_get_type(s->fmt->format) == AST_MEDIA_TYPE_AUDIO) {
+		if (ast_channel_stream(chan))
+			ast_closestream(ast_channel_stream(chan));
+		ast_channel_stream_set(chan, s);
+	} else {
+		if (ast_channel_vstream(chan))
+			ast_closestream(ast_channel_vstream(chan));
+		ast_channel_vstream_set(chan, s);
+	}
+	return 1; /* found */
+}
+
+int stream_mediacache_file(const char *uri, const char *filename, struct ast_channel *chan) {
+	char ext[PATH_MAX];
+	char fullpath[PATH_MAX];
+	struct ast_format_def *f;
+	int format_found = 0;
+	
+	ast_media_cache_retrieve_metadata(uri, "ext", ext, PATH_MAX);
+	snprintf(fullpath, PATH_MAX, "%s%s", filename, ext);
+
+	/* lookup the format */
+	AST_RWLIST_RDLOCK(&formats);
+	AST_RWLIST_TRAVERSE(&formats, f, list)
+	{
+		if (exts_compare(f->exts, ext + 1)) {
+			format_found = 1;
+			break;
+		}
+	}
+	AST_RWLIST_UNLOCK(&formats);
+
+	if (!format_found) {
+		ast_log(LOG_WARNING, "No format found for extension %s\n", ext+1);
+		return 0;
+	}
+
+	return stream_file(fullpath, chan, f);
+}
+
 /*!
  * \internal
  * \brief perform various actions on a file. Second argument
@@ -578,13 +643,12 @@
 				ast_free(fn);
 				continue;
 			}
+
 			/* for 'OPEN' we need to be sure that the format matches
 			 * what the channel can process
 			 */
 			if (action == ACTION_OPEN) {
 				struct ast_channel *chan = (struct ast_channel *)arg2;
-				FILE *bfile;
-				struct ast_filestream *s;
 
 				if ((ast_format_cmp(ast_channel_writeformat(chan), f->format) == AST_FORMAT_CMP_NOT_EQUAL) &&
 				     !(((ast_format_get_type(f->format) == AST_MEDIA_TYPE_AUDIO) && fmt) ||
@@ -592,40 +656,11 @@
 					ast_free(fn);
 					continue;	/* not a supported format */
 				}
-				if ( (bfile = fopen(fn, "r")) == NULL) {
-					ast_free(fn);
-					continue;	/* cannot open file */
-				}
-				s = get_filestream(f, bfile);
-				if (!s) {
-					fclose(bfile);
-					ast_free(fn);	/* cannot allocate descriptor */
+				res = stream_file(fn, chan, f);
+				ast_free(fn);
+				if (!res) {
 					continue;
 				}
-				if (open_wrapper(s)) {
-					ast_free(fn);
-					ast_closestream(s);
-					continue;	/* cannot run open on file */
-				}
-				if (st.st_size == 0) {
-					ast_log(LOG_WARNING, "File %s detected to have zero size.\n", fn);
-				}
-				/* ok this is good for OPEN */
-				res = 1;	/* found */
-				s->lasttimeout = -1;
-				s->fmt = f;
-				s->trans = NULL;
-				s->filename = NULL;
-				if (ast_format_get_type(s->fmt->format) == AST_MEDIA_TYPE_AUDIO) {
-					if (ast_channel_stream(chan))
-						ast_closestream(ast_channel_stream(chan));
-					ast_channel_stream_set(chan, s);
-				} else {
-					if (ast_channel_vstream(chan))
-						ast_closestream(ast_channel_vstream(chan));
-					ast_channel_vstream_set(chan, s);
-				}
-				ast_free(fn);
 				break;
 			}
 			switch (action) {
@@ -695,10 +730,6 @@
 		return 0;
 	}
 
-	if (is_remote_path(filename) && !ast_media_cache_retrieve(filename, NULL, buf, buflen)) {
-		return filehelper(buf, result_cap, NULL, ACTION_EXISTS);
-	}
-
 	if (ast_language_is_prefix && !is_absolute_path(filename)) { /* new layout */
 		if (lang) {
 			snprintf(buf, buflen, "%s/%s", lang, filename);
@@ -787,6 +818,35 @@
 	return 0;
 }
 
+int get_mediacache_file(const char *uri, char *file_path, size_t len, struct ast_format_cap *file_fmt_cap) {
+	struct ast_format* format;
+	char ext[PATH_MAX];
+	
+	/* download / find file */
+	if (ast_media_cache_retrieve(uri, NULL, file_path, len)) {
+		ast_log(LOG_WARNING, "Could not retrieve uri %s from media cache\n", uri);
+		return 0;
+	}
+
+	/*
+	 * we have a local filename without extension
+	 * retrieve metadata for format
+	 */
+	if (ast_media_cache_retrieve_metadata(uri, "ext", ext, PATH_MAX)) {
+		ast_log(LOG_WARNING, "No extension in media cache for file %s\n", uri);
+		return 0;
+	}
+	
+	format = ast_get_format_for_file_ext(ext+1);
+	if (!format) {
+		ast_log(LOG_WARNING, "Could not retrieve format for file extension %s\n", ext);
+		return 0;
+	}
+	ast_format_cap_append(file_fmt_cap, format, 0);
+	ast_debug(1, "Retrieved uri %s to local file %s (extension %s)\n", uri, file_path, ext);
+	return 1;
+}
+
 struct ast_filestream *ast_openstream(struct ast_channel *chan, const char *filename, const char *preflang)
 {
 	return ast_openstream_full(chan, filename, preflang, 0);
@@ -803,6 +863,7 @@
 	int res;
 	int buflen;
 	char *buf;
+	int from_mediacache;
 
 	if (!asis) {
 		/* do this first, otherwise we detect the wrong writeformat */
@@ -818,9 +879,13 @@
 	if (!(file_fmt_cap = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT))) {
 		return NULL;
 	}
-	if (!fileexists_core(filename, NULL, preflang, buf, buflen, file_fmt_cap) ||
-		!ast_format_cap_has_type(file_fmt_cap, AST_MEDIA_TYPE_AUDIO)) {
-
+	
+	if (is_remote_path(filename) && get_mediacache_file(filename, buf, buflen, file_fmt_cap)) {
+		from_mediacache = 1;
+	} else if (fileexists_core(filename, NULL, preflang, buf, buflen, file_fmt_cap) &&
+			ast_format_cap_has_type(file_fmt_cap, AST_MEDIA_TYPE_AUDIO)) {
+		from_mediacache = 0;
+	} else {
 		ast_log(LOG_WARNING, "File %s does not exist in any format\n", filename);
 		ao2_ref(file_fmt_cap, -1);
 		return NULL;
@@ -838,7 +903,11 @@
 	if (res == -1) {	/* No format available that works with this channel */
 		return NULL;
 	}
-	res = filehelper(buf, chan, NULL, ACTION_OPEN);
+	if (from_mediacache) {
+		res = stream_mediacache_file(filename, buf, chan);
+	} else {
+		res = filehelper(buf, chan, NULL, ACTION_OPEN);
+	}
 	if (res >= 0)
 		return ast_channel_stream(chan);
 	return NULL;

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I89ad019b4344a8f2f09d388740f78751a8e6673b
Gerrit-Change-Number: 19892
Gerrit-PatchSet: 1
Gerrit-Owner: Nathan Bruning <nathan at iperity.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20230208/c68f45aa/attachment-0001.html>


More information about the asterisk-code-review mailing list