[AdaptiveTree] Removed some uses of RemoveParameters

The current RemoveParameters method can be misleading and can delete required url parts
xbmc · Aug 25, 2023 · 2793909 · 2793909
1 parent f6814cd
commit 2793909
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 17 deletions.
diff --git a/src/parser/DASHTree.cpp b/src/parser/DASHTree.cpp
@@ -98,7 +98,7 @@ bool adaptive::CDashTree::Open(std::string_view url,
 
   m_manifestRespHeaders = headers;
   manifest_url_ = url;
-  base_url_ = URL::RemoveParameters(url.data());
+  base_url_ = URL::GetUrlPath(url.data());
 
   if (!ParseManifest(data))
     return false;
@@ -1514,7 +1514,7 @@ void adaptive::CDashTree::RefreshLiveSegments()
   {
     manifestUrl = manifest_url_;
     if (!manifestParams.empty())
-      manifestUrl = URL::RemoveParameters(manifestUrl, false);
+      manifestUrl = URL::RemoveParameters(manifestUrl);
   }
   else
     manifestUrl = location_;

diff --git a/src/parser/HLSTree.cpp b/src/parser/HLSTree.cpp
@@ -151,7 +151,7 @@ bool adaptive::CHLSTree::Open(std::string_view url,
   SaveManifest(nullptr, data, url);
 
   manifest_url_ = url;
-  base_url_ = URL::RemoveParameters(url.data());
+  base_url_ = URL::GetUrlPath(url.data());
 
   if (!ParseManifest(data))
   {
@@ -205,7 +205,7 @@ PLAYLIST::PrepareRepStatus adaptive::CHLSTree::prepareRepresentation(PLAYLIST::C
 
     SaveManifest(adp, resp.data, manifestUrl);
 
-    rep->SetBaseUrl(URL::RemoveParameters(resp.effectiveUrl));
+    rep->SetBaseUrl(URL::GetUrlPath(resp.effectiveUrl));
 
     EncryptionType currentEncryptionType = EncryptionType::CLEAR;
 
@@ -354,7 +354,7 @@ PLAYLIST::PrepareRepStatus adaptive::CHLSTree::prepareRepresentation(PLAYLIST::C
         if (rep->GetContainerType() == ContainerType::NOTYPE)
         {
           // Try find the container type on the representation according to the file extension
-          std::string url = URL::RemoveParameters(line, false);
+          std::string url = URL::GetUrlPath(line);
           // Remove domain on absolute url, to not confuse top-level domain as extension
           url = url.substr(URL::GetDomainUrl(url).size());
 

diff --git a/src/parser/SmoothTree.cpp b/src/parser/SmoothTree.cpp
@@ -33,7 +33,7 @@ bool adaptive::CSmoothTree::Open(std::string_view url,
   SaveManifest("", data, "");
 
   manifest_url_ = url;
-  base_url_ = URL::RemoveParameters(url.data());
+  base_url_ = URL::GetUrlPath(url.data());
 
   if (!ParseManifest(data))
     return false;

diff --git a/src/utils/UrlUtils.cpp b/src/utils/UrlUtils.cpp
@@ -164,18 +164,32 @@ std::string UTILS::URL::GetParameters(std::string& url) {
   return "";
 }
 
-std::string UTILS::URL::RemoveParameters(std::string url, bool removeFilenameParam /* = true */)
+std::string UTILS::URL::RemoveParameters(std::string url)
 {
   size_t paramsPos = url.find('?');
   if (paramsPos != std::string::npos)
     url.resize(paramsPos);
 
-  if (removeFilenameParam)
+  return url;
+}
+
+std::string UTILS::URL::GetUrlPath(std::string url)
+{
+  if (url.empty())
+    return url;
+
+  size_t paramsPos = url.find('?');
+  if (paramsPos != std::string::npos)
+    url.resize(paramsPos);
+
+  // The part of the base url after last / is not a directory so will not be taken into account
+  if (url.back() != '/')
   {
-    size_t slashPos = url.find_last_of('/');
-    if (slashPos != std::string::npos && slashPos != (url.find("://") + 2))
-      url.resize(slashPos + 1);
+    size_t slashPos = url.rfind("/");
+    if (slashPos > url.find("://") + 3)
+      url.erase(slashPos + 1);
   }
+
   return url;
 }
 

diff --git a/src/utils/UrlUtils.h b/src/utils/UrlUtils.h
@@ -53,13 +53,20 @@ std::string GetParametersFromPlaceholder(std::string& url, std::string_view plac
  */
 std::string GetParameters(std::string& url);
 
-/*! \brief Remove URL parameters e.g. "?q=something"
- *  \param url An URL
- *  \param removeFilenameParam If true remove the last URL level if it
- *   contains a filename with extension
- *  \return The URL without parameters
+/*!
+ * \brief Remove URL parameters e.g. "?q=something"
+ * \param url An URL
+ * \return The URL without parameters
+ */
+std::string RemoveParameters(std::string url);
+
+/*!
+ * \brief Get the url path, by removing file part and parameters
+ *        e.g. https://sample.com/part1/part2?test become https://sample.com/part1/
+ * \param url An URL
+ * \return The URL path
  */
-std::string RemoveParameters(std::string url, bool removeFilenameParam = true);
+std::string GetUrlPath(std::string url);
 
 /*! \brief Append a string of parameters to an URL with/without pre-existents params
  *  \param url URL where append the parameters