Skip to content

Commit 7b29fab

Browse files
authored
Fix path_in_repo validation when committing files (#1382)
* Fix invalid path_in_repo in CommitOperationAdd * raise if absolute path_in_repo in CommitOperationAdd * Validate path_in_repo for CommitOperationDelete as well
1 parent bf0f208 commit 7b29fab

File tree

3 files changed

+50
-0
lines changed

3 files changed

+50
-0
lines changed

src/huggingface_hub/_commit_api.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ class CommitOperationDelete:
5656
is_folder: Union[bool, Literal["auto"]] = "auto"
5757

5858
def __post_init__(self):
59+
self.path_in_repo = _validate_path_in_repo(self.path_in_repo)
60+
5961
if self.is_folder == "auto":
6062
self.is_folder = self.path_in_repo.endswith("/")
6163
if not isinstance(self.is_folder, bool):
@@ -95,6 +97,8 @@ class CommitOperationAdd:
9597

9698
def __post_init__(self) -> None:
9799
"""Validates `path_or_fileobj` and compute `upload_info`."""
100+
self.path_in_repo = _validate_path_in_repo(self.path_in_repo)
101+
98102
# Validate `path_or_fileobj` value
99103
if isinstance(self.path_or_fileobj, Path):
100104
self.path_or_fileobj = str(self.path_or_fileobj)
@@ -194,6 +198,17 @@ def b64content(self) -> bytes:
194198
return base64.b64encode(file.read())
195199

196200

201+
def _validate_path_in_repo(path_in_repo: str) -> str:
202+
# Validate `path_in_repo` value to prevent a server-side issue
203+
if path_in_repo.startswith("/"):
204+
path_in_repo = path_in_repo[1:]
205+
if path_in_repo == "." or path_in_repo == ".." or path_in_repo.startswith("../"):
206+
raise ValueError(f"Invalid `path_in_repo` in CommitOperation: '{path_in_repo}'")
207+
if path_in_repo.startswith("./"):
208+
path_in_repo = path_in_repo[2:]
209+
return path_in_repo
210+
211+
197212
CommitOperation = Union[CommitOperationAdd, CommitOperationDelete]
198213

199214

tests/test_commit_api.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,30 @@ def test_is_folder_wrong_value(self):
3232
CommitOperationDelete(path_in_repo="path/to/folder", is_folder="any value")
3333

3434

35+
class TestCommitOperationPathInRepo(unittest.TestCase):
36+
valid_values = { # key is input, value is expected validated output
37+
"file.txt": "file.txt",
38+
".file.txt": ".file.txt",
39+
"/file.txt": "file.txt",
40+
"./file.txt": "file.txt",
41+
}
42+
invalid_values = [".", "..", "../file.txt"]
43+
44+
def test_path_in_repo_valid(self) -> None:
45+
for input, expected in self.valid_values.items():
46+
with self.subTest(f"Testing with valid input: '{input}'"):
47+
self.assertEqual(CommitOperationAdd(path_in_repo=input, path_or_fileobj=b"").path_in_repo, expected)
48+
self.assertEqual(CommitOperationDelete(path_in_repo=input).path_in_repo, expected)
49+
50+
def test_path_in_repo_invalid(self) -> None:
51+
for input in self.invalid_values:
52+
with self.subTest(f"Testing with invalid input: '{input}'"):
53+
with self.assertRaises(ValueError):
54+
CommitOperationAdd(path_in_repo=input, path_or_fileobj=b"")
55+
with self.assertRaises(ValueError):
56+
CommitOperationDelete(path_in_repo=input)
57+
58+
3559
class TestWarnOnOverwritingOperations(unittest.TestCase):
3660
add_file_ab = CommitOperationAdd(path_in_repo="a/b.txt", path_or_fileobj=b"data")
3761
add_file_abc = CommitOperationAdd(path_in_repo="a/b/c.md", path_or_fileobj=b"data")

tests/test_hf_api.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1980,6 +1980,17 @@ def test_allow_txt_not_root_ignore_subdir(self):
19801980
{"sub/file.txt"}, # only .txt files, not in subdir, not at root
19811981
)
19821982

1983+
def test_path_in_repo_dot(self):
1984+
"""Regression test for #1382 when using `path_in_repo="."`.
1985+
1986+
Using `path_in_repo="."` or `path_in_repo=None` should be equivalent.
1987+
See https://github.com/huggingface/huggingface_hub/pull/1382.
1988+
"""
1989+
operation_with_dot = self._upload_folder_alias(path_in_repo=".", allow_patterns=["file.txt"])[0]
1990+
operation_with_none = self._upload_folder_alias(path_in_repo=None, allow_patterns=["file.txt"])[0]
1991+
self.assertEqual(operation_with_dot.path_in_repo, "file.txt")
1992+
self.assertEqual(operation_with_none.path_in_repo, "file.txt")
1993+
19831994
def test_delete_txt(self):
19841995
operations = self._upload_folder_alias(delete_patterns="*.txt")
19851996
added_files = {op.path_in_repo for op in operations if isinstance(op, CommitOperationAdd)}

0 commit comments

Comments
 (0)