Repository: huggingface/huggingface_hub
Pull request #1164 (2022-11-07 17:03:24)

FIX url encoding in hf_hub_url
Fix https://github.com/huggingface/huggingface_hub/issues/1161. Bug doesn't seem to be elsewhere than in `hf_hub_url` itself.
Added 2 regression tests just to be sure. 

cc @philschmid thanks for reporting it :)

(just wondering for myself: who really thought it was a good idea to have a `?` in a filename ??? :smile:)

HuggingFaceDocBuilderDev: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/huggingface_hub/pr_1164). All of your documentation changes will be reflected on that endpoint.

Patch:
diff --git a/src/huggingface_hub/file_download.py b/src/huggingface_hub/file_download.py
index 92f041f4..c4334efb 100644
--- a/src/huggingface_hub/file_download.py
+++ b/src/huggingface_hub/file_download.py
@@ -234,7 +234,7 @@ def hf_hub_url(
     return HUGGINGFACE_CO_URL_TEMPLATE.format(
         repo_id=repo_id,
         revision=quote(revision, safe=""),
-        filename=filename,
+        filename=quote(filename),
     )
 
 


Test patch:
diff --git a/tests/test_file_download.py b/tests/test_file_download.py
index 9ae86fb4..aa029a80 100644
--- a/tests/test_file_download.py
+++ b/tests/test_file_download.py
@@ -397,6 +397,49 @@ class StagingCachedDownloadTest(unittest.TestCase):
                 )
 
 
+@pytest.mark.usefixtures("fx_cache_dir")
+class StagingCachedDownloadOnAwfulFilenamesTest(unittest.TestCase):
+    """Implement regression tests for #1161.
+
+    Issue was on filename not url encoded by `hf_hub_download` and `hf_hub_url`.
+
+    See https://github.com/huggingface/huggingface_hub/issues/1161
+    """
+
+    cache_dir: Path
+    repo_id = "valid_org/repo_with_awful_filename"
+    subfolder = "subfolder/to?"
+    filename = "awful?filename%you:should,never.give"
+    filepath = "subfolder/to?/awful?filename%you:should,never.give"
+    expected_url = "https://hub-ci.huggingface.co/valid_org/repo_with_awful_filename/resolve/main/subfolder/to%3F/awful%3Ffilename%25you%3Ashould%2Cnever.give"
+
+    def test_hf_hub_url_on_awful_filepath(self):
+        self.assertEqual(hf_hub_url(self.repo_id, self.filepath), self.expected_url)
+
+    def test_hf_hub_url_on_awful_subfolder_and_filename(self):
+        self.assertEqual(
+            hf_hub_url(self.repo_id, self.filename, subfolder=self.subfolder),
+            self.expected_url,
+        )
+
+    def test_hf_hub_download_on_awful_filepath(self):
+        local_path = hf_hub_download(
+            self.repo_id, self.filepath, cache_dir=self.cache_dir
+        )
+        # Local path is not url-encoded
+        self.assertTrue(local_path.endswith(self.filepath))
+
+    def test_hf_hub_download_on_awful_subfolder_and_filename(self):
+        local_path = hf_hub_download(
+            self.repo_id,
+            self.filename,
+            subfolder=self.subfolder,
+            cache_dir=self.cache_dir,
+        )
+        # Local path is not url-encoded
+        self.assertTrue(local_path.endswith(self.filepath))
+
+
 class CreateSymlinkTest(unittest.TestCase):
     @patch("huggingface_hub.file_download.are_symlinks_supported")
     def test_create_relative_symlink_concurrent_access(