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

[Nathan Bruning]

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


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.

ASTERISK-30408 #close

Change-Id: If6b5f4af9cbdf39009252594eb987b07bfb493a4
---
M include/asterisk/bucket.h
M main/bucket.c
2 files changed, 70 insertions(+), 7 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/97/19797/1

diff --git a/include/asterisk/bucket.h b/include/asterisk/bucket.h
index 47cc561..7765c22 100644
--- a/include/asterisk/bucket.h
+++ b/include/asterisk/bucket.h
@@ -454,6 +454,16 @@
 struct ast_json *ast_bucket_file_json(const struct ast_bucket_file *file);
 
 /*!
+ * \brief Return 1 is there are any files with the given glob pattern
+ * 
+ * \param pattern glob pattern
+ * 
+ * \retval 1 if any files match
+ * \retval 0 if not (or error)
+ */
+int files_exist_with_pattern(const char *pattern);
+
+/*!
  * \brief Common file snapshot creation callback for creating a temporary file
  *
  * \param file Pointer to the file snapshot
diff --git a/main/bucket.c b/main/bucket.c
index 01934f3..10a4453 100644
--- a/main/bucket.c
+++ b/main/bucket.c
@@ -896,19 +896,57 @@
 	return json;
 }
 
+/* return 1 if a file with the given pattern exists, 0 otherwise. On error, 0 is returned. */
+int files_exist_with_pattern(const char *pattern) {
+	int glob_ret;
+	glob_t globbuf;
+
+	globbuf.gl_offs = 0;	/* initialize it to silence gcc */
+	glob_ret = glob(pattern, MY_GLOB_FLAGS, NULL, &globbuf);
+	globfree(&globbuf);
+
+	if (glob_ret == 0) {
+		/* glob succeeded and there are matches */
+		return 1;
+	} else if (glob_ret == GLOB_NOMATCH) {
+		return 0;
+	} else {
+		/* error during matching, return 0 for no match */
+		return 0;
+	}
+}
+
 int ast_bucket_file_temporary_create(struct ast_bucket_file *file)
 {
 	int fd;
+	int i;
+	char pattern[PATH_MAX+2];
 
-	snprintf(file->path, sizeof(file->path), "%s/bucket-XXXXXX", ast_config_AST_CACHE_DIR);
+	/* limit number of tries to avoid infinite loop if logic fails for some reason */
+	for(i=0; i<100; i++) {
+		snprintf(file->path, sizeof(file->path), "%s/bucket-XXXXXX", ast_config_AST_CACHE_DIR);
 
-	fd = mkstemp(file->path);
-	if (fd < 0) {
-		return -1;
+		fd = mkstemp(file->path);
+		if (fd < 0) {
+			return -1;
+		}
+		close(fd);
+
+		/* Asterisk media lookup will try different file extensions (wav, wav16) depending on the 
+		 * channel format. To avoid conflicts, each bucket entry should have a unique filename -
+		 * not considering extensions. Ie: if there is a file x.wav, we shouldn't create x.wav16.
+		 * (note that at this point, file->path is still without extension)
+		 */
+		snprintf(pattern, sizeof(pattern), "%s.*", file->path);
+		if (!files_exist_with_pattern(pattern)) {
+			/* safe to return this filename */
+			return 0;
+		}
+		unlink(file->path);
+		ast_log(LOG_NOTICE, "Media cache filename %s already in-use, retrying\n", file->path);
 	}
-
-	close(fd);
-	return 0;
+	ast_log(LOG_ERROR, "Could not decide on temp filename after 100 tries, giving up\n");
+	return -1;
 }
 
 void ast_bucket_file_temporary_destroy(struct ast_bucket_file *file)

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: If6b5f4af9cbdf39009252594eb987b07bfb493a4
Gerrit-Change-Number: 19797
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/20230127/0d350658/attachment.html>