[Asterisk-code-review] res_http_media_cache: Cleanup audio format lookup in HTTP requests (asterisk[master])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/16215 )

Change subject: res_http_media_cache: Cleanup audio format lookup in HTTP requests
......................................................................

res_http_media_cache: Cleanup audio format lookup in HTTP requests

Asterisk first looks at the end of the URL to determine the file
extension of the returned audio, which in many cases will not work
because the URL may end with a query string or a URL fragment. If that
fails, Asterisk then looks at the Content-Type header and then finally
parses the URL to get the extension.

The order has been changed such that we look at the Content-Type
header first, followed by looking for the extension of the parsed
URL. We no longer look at the end of the URL, which was error prone.

ASTERISK-29527 #close

Change-Id: I1e3f83b339ef2b80661704717c23568536511032
---
A doc/UPGRADE-staging/http-media-cache-lookup-order.txt
M res/res_http_media_cache.c
2 files changed, 13 insertions(+), 14 deletions(-)

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



diff --git a/doc/UPGRADE-staging/http-media-cache-lookup-order.txt b/doc/UPGRADE-staging/http-media-cache-lookup-order.txt
new file mode 100644
index 0000000..83c31dc
--- /dev/null
+++ b/doc/UPGRADE-staging/http-media-cache-lookup-order.txt
@@ -0,0 +1,9 @@
+Subject: res_http_media_cache
+
+When fetching a file for playback from a URL, Asterisk will now first
+use the value of the Content-Type header in the HTTP response to
+determine the format of the audio data, and only if it is unable to do
+that will it attempt to parse the URL and extract the extension from
+the path portion. Previously Asterisk would first look at the end of
+the URL, which may have included query string parameters or a URL
+fragment, which was error prone.
diff --git a/res/res_http_media_cache.c b/res/res_http_media_cache.c
index 5410566..9f560c7 100644
--- a/res/res_http_media_cache.c
+++ b/res/res_http_media_cache.c
@@ -171,11 +171,6 @@
 	return NULL;
 }
 
-static char *file_extension_from_url(struct ast_bucket_file *bucket_file, char *buffer, size_t capacity)
-{
-	return file_extension_from_string(ast_sorcery_object_get_id(bucket_file), buffer, capacity);
-}
-
 /*!
  * \internal
  * \brief Normalize the value of a Content-Type header
@@ -249,18 +244,13 @@
 
 static void bucket_file_set_extension(struct ast_bucket_file *bucket_file)
 {
-	/* We will attempt to determine an extension in the following order for backwards
-	 * compatibility:
-	 *
-	 * 1. Look at tail end of URL for extension
-	 * 2. Use the Content-Type header if present
-	 * 3. Parse the URL (assuming we can) and look at the tail of the path
-	 */
+	/* Using Content-Type first allows for the most flexibility for whomever
+	 * is serving up the audio file. If that doesn't turn up anything useful
+	 * we'll try to parse the URL and use the extension */
 
 	char buffer[64];
 
-	if (file_extension_from_url(bucket_file, buffer, sizeof(buffer))
-	   || file_extension_from_content_type(bucket_file, buffer, sizeof(buffer))
+	if (file_extension_from_content_type(bucket_file, buffer, sizeof(buffer))
 	   || file_extension_from_url_path(bucket_file, buffer, sizeof(buffer))) {
 		ast_bucket_file_metadata_set(bucket_file, "ext", buffer);
 	}

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I1e3f83b339ef2b80661704717c23568536511032
Gerrit-Change-Number: 16215
Gerrit-PatchSet: 2
Gerrit-Owner: Sean Bright <sean at seanbright.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210802/2332cf4a/attachment.html>