From 98b44d01df6a952466b5a1667818da877502da97 Mon Sep 17 00:00:00 2001
From: Cosmin Stejerean <cstejerean@meta.com>
Date: Fri, 10 May 2024 17:27:13 -0700
Subject: [PATCH] fix: escape media URLs in MPD (#1395)

Currently `media_info.media_file_url()` is not escaped when placed into
MPD for things like BaseURL. This for example breaks when trying to us a
file name that contains special characters like &. Since these are
supposed to be URLs let's URL encode them.

Fixes #1107

---------

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
---
 packager/mpd/CMakeLists.txt       |  1 +
 packager/mpd/base/mpd_builder.cc  |  2 +-
 packager/mpd/base/xml/xml_node.cc | 26 ++++++++++++++++++++++++--
 packager/mpd/base/xml/xml_node.h  |  4 ++++
 4 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/packager/mpd/CMakeLists.txt b/packager/mpd/CMakeLists.txt
index 48776615d5..8284d467e6 100644
--- a/packager/mpd/CMakeLists.txt
+++ b/packager/mpd/CMakeLists.txt
@@ -51,6 +51,7 @@ target_link_libraries(mpd_builder
   media_base
   mpd_media_info_proto
   utils_clock
+  libcurl
 )
 
 
diff --git a/packager/mpd/base/mpd_builder.cc b/packager/mpd/base/mpd_builder.cc
index 0ebe40dc0f..2533fc01e3 100644
--- a/packager/mpd/base/mpd_builder.cc
+++ b/packager/mpd/base/mpd_builder.cc
@@ -167,7 +167,7 @@ std::optional<xml::XmlNode> MpdBuilder::GenerateMpd() {
   // Add baseurls to MPD.
   for (const std::string& base_url : base_urls_) {
     XmlNode xml_base_url("BaseURL");
-    xml_base_url.SetContent(base_url);
+    xml_base_url.SetUrlEncodedContent(base_url);
 
     if (!mpd.AddChild(std::move(xml_base_url)))
       return std::nullopt;
diff --git a/packager/mpd/base/xml/xml_node.cc b/packager/mpd/base/xml/xml_node.cc
index 6da265054b..ff3c9d65a5 100644
--- a/packager/mpd/base/xml/xml_node.cc
+++ b/packager/mpd/base/xml/xml_node.cc
@@ -18,6 +18,7 @@
 #include <absl/strings/escaping.h>
 #include <absl/strings/numbers.h>
 #include <absl/strings/str_format.h>
+#include <curl/curl.h>
 #include <libxml/tree.h>
 
 #include <packager/macros/compiler.h>
@@ -53,6 +54,18 @@ const char kDTSCCodec[] = "dtsc";
 const char kDTSECodec[] = "dtse";
 const char kDTSXCodec[] = "dtsx";
 
+std::string urlEncode(const std::string& input) {
+  // NOTE: According to the docs, "Since 7.82.0, the curl parameter is ignored".
+  CURL* curl = NULL;
+  char* output = curl_easy_escape(curl, input.c_str(), input.length());
+  if (output) {
+    std::string encodedUrl(output);
+    curl_free(output);  // Free the output string when done
+    return encodedUrl;
+  }
+  return "";  // Return empty string if initialization fails
+}
+
 std::string RangeToString(const Range& range) {
   return absl::StrFormat("%u-%u", range.begin(), range.end());
 }
@@ -220,11 +233,19 @@ void XmlNode::AddContent(const std::string& content) {
   xmlNodeAddContent(impl_->node.get(), BAD_CAST content.c_str());
 }
 
+void XmlNode::AddUrlEncodedContent(const std::string& content) {
+  AddContent(urlEncode(content));
+}
+
 void XmlNode::SetContent(const std::string& content) {
   DCHECK(impl_->node);
   xmlNodeSetContent(impl_->node.get(), BAD_CAST content.c_str());
 }
 
+void XmlNode::SetUrlEncodedContent(const std::string& content) {
+  SetContent(urlEncode(content));
+}
+
 std::set<std::string> XmlNode::ExtractReferencedNamespaces() const {
   std::set<std::string> namespaces;
   TraverseNodesAndCollectNamespaces(impl_->node.get(), &namespaces);
@@ -400,7 +421,7 @@ bool RepresentationXmlNode::AddVODOnlyInfo(const MediaInfo& media_info,
 
   if (media_info.has_media_file_url() && !use_single_segment_url_with_media) {
     XmlNode base_url("BaseURL");
-    base_url.SetContent(media_info.media_file_url());
+    base_url.SetUrlEncodedContent(media_info.media_file_url());
 
     RCHECK(AddChild(std::move(base_url)));
   }
@@ -452,7 +473,8 @@ bool RepresentationXmlNode::AddVODOnlyInfo(const MediaInfo& media_info,
 
   if (use_single_segment_url_with_media) {
     XmlNode media_url("SegmentURL");
-    RCHECK(media_url.SetStringAttribute("media", media_info.media_file_url()));
+    RCHECK(media_url.SetStringAttribute(
+        "media", urlEncode(media_info.media_file_url())));
     RCHECK(child.AddChild(std::move(media_url)));
   }
 
diff --git a/packager/mpd/base/xml/xml_node.h b/packager/mpd/base/xml/xml_node.h
index c73c0cbe82..62a7d77814 100644
--- a/packager/mpd/base/xml/xml_node.h
+++ b/packager/mpd/base/xml/xml_node.h
@@ -83,6 +83,8 @@ class XmlNode {
   /// Similar to SetContent, but appends to the end of existing content.
   void AddContent(const std::string& content);
 
+  void AddUrlEncodedContent(const std::string& content);
+
   /// Set the contents of an XML element using a string.
   /// This cannot set child elements because <> will become &lt; and &rt;
   /// This should be used to set the text for the element, e.g. setting
@@ -91,6 +93,8 @@ class XmlNode {
   ///        be added to the element.
   void SetContent(const std::string& content);
 
+  void SetUrlEncodedContent(const std::string& content);
+
   /// @return namespaces used in the node and its descendents.
   std::set<std::string> ExtractReferencedNamespaces() const;