Repository: huggingface/huggingface_hub
Pull request #1428 (2023-04-06 04:44:22)

Normalize both possible variants of the Etag to remove potentially invalid path elements
Some hf web servers are returning Etags prefaced with W/ which leads to some derived filenames containing quotation marks due to inadequate normalizing.

dwforbes: This relates to both issue #1423 and #1426 
julien-c: i wouldn't merge this because it's "spec" that Etags (and linked-etags) are sha1 and sha256 of the corresponding files
HuggingFaceDocBuilderDev: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/huggingface_hub/pr_1428). All of your documentation changes will be reflected on that endpoint.
Wauplin: @dwforbes I completed your PR to add tests. If a similar error happens in the future, `huggingface_hub` should now be able to handle it.

@julien-c I added tests to ensure that if a weak reference is passed in the future, it has the same target as the current (strong) reference. I hardcoded etags from https://huggingface.co/gpt2 for both regular and LFS files.

Patch:
diff --git a/src/huggingface_hub/file_download.py b/src/huggingface_hub/file_download.py
index 9d594cce..4e28c5e6 100644
--- a/src/huggingface_hub/file_download.py
+++ b/src/huggingface_hub/file_download.py
@@ -813,7 +813,8 @@ def _normalize_etag(etag: Optional[str]) -> Optional[str]:
       ETag: W/"<etag_value>"
       ETag: "<etag_value>"
 
-    The hf.co hub guarantees to only send the second form.
+    For now, we only expect the second form from the server, but we want to be future-proof so we support both. For
+    more context, see `TestNormalizeEtag` tests and https://github.com/huggingface/huggingface_hub/pull/1428.
 
     Args:
         etag (`str`, *optional*): HTTP header
@@ -824,7 +825,7 @@ def _normalize_etag(etag: Optional[str]) -> Optional[str]:
     """
     if etag is None:
         return None
-    return etag.strip('"')
+    return etag.lstrip("W/").strip('"')
 
 
 def _create_relative_symlink(src: str, dst: str, new_blob: bool = False) -> None:


Test patch:
diff --git a/tests/test_file_download.py b/tests/test_file_download.py
index 1a9b8d84..9e84af7a 100644
--- a/tests/test_file_download.py
+++ b/tests/test_file_download.py
@@ -21,11 +21,13 @@ from unittest.mock import Mock, patch
 
 import pytest
 import requests
+from requests import Response
 
 import huggingface_hub.file_download
 from huggingface_hub import HfApi
 from huggingface_hub.constants import (
     CONFIG_NAME,
+    HUGGINGFACE_HEADER_X_LINKED_ETAG,
     PYTORCH_WEIGHTS_NAME,
     REPO_TYPE_DATASET,
 )
@@ -33,6 +35,7 @@ from huggingface_hub.file_download import (
     _CACHED_NO_EXIST,
     _create_symlink,
     _get_pointer_path,
+    _normalize_etag,
     _request_wrapper,
     _to_local_dir,
     cached_download,
@@ -889,6 +892,46 @@ class CreateSymlinkTest(unittest.TestCase):
         shutil.rmtree(test_dir)
 
 
+class TestNormalizeEtag(unittest.TestCase):
+    """Unit tests implemented after a server-side change broke the ETag normalization once (see #1428).
+
+    TL;DR: _normalize_etag was expecting only strong references, but the server started to return weak references after
+    a config update. Problem was quickly fixed server-side but we prefer to make sure this doesn't happen again by
+    supporting weak etags. For context, etags are used to build the cache-system structure.
+
+    For more details, see https://github.com/huggingface/huggingface_hub/pull/1428 and related issues.
+    """
+
+    def test_strong_reference(self):
+        self.assertEqual(
+            _normalize_etag('"a16a55fda99d2f2e7b69cce5cf93ff4ad3049930"'), "a16a55fda99d2f2e7b69cce5cf93ff4ad3049930"
+        )
+
+    def test_weak_reference(self):
+        self.assertEqual(
+            _normalize_etag('W/"a16a55fda99d2f2e7b69cce5cf93ff4ad3049930"'), "a16a55fda99d2f2e7b69cce5cf93ff4ad3049930"
+        )
+
+    @with_production_testing
+    def test_resolve_endpoint_on_regular_file(self):
+        url = "https://huggingface.co/gpt2/resolve/e7da7f221d5bf496a48136c0cd264e630fe9fcc8/README.md"
+        response = requests.head(url)
+        self.assertEqual(self._get_etag_and_normalize(response), "a16a55fda99d2f2e7b69cce5cf93ff4ad3049930")
+
+    @with_production_testing
+    def test_resolve_endpoint_on_lfs_file(self):
+        url = "https://huggingface.co/gpt2/resolve/e7da7f221d5bf496a48136c0cd264e630fe9fcc8/pytorch_model.bin"
+        response = requests.head(url)
+        self.assertEqual(
+            self._get_etag_and_normalize(response), "7c5d3f4b8b76583b422fcb9189ad6c89d5d97a094541ce8932dce3ecabde1421"
+        )
+
+    @staticmethod
+    def _get_etag_and_normalize(response: Response) -> str:
+        response.raise_for_status()
+        return _normalize_etag(response.headers.get(HUGGINGFACE_HEADER_X_LINKED_ETAG) or response.headers.get("ETag"))
+
+
 def _recursive_chmod(path: str, mode: int) -> None:
     # Taken from https://stackoverflow.com/a/2853934
     for root, dirs, files in os.walk(path):