Skip to content

Commit

Permalink
Merge pull request #1672 from trail-of-forks/robust-refname-checks
Browse files Browse the repository at this point in the history
Add more checks for the validity of refnames
  • Loading branch information
Byron authored Sep 22, 2023
2 parents 1774f1e + 46d3d05 commit e98f57b
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 2 deletions.
50 changes: 48 additions & 2 deletions git/refs/symbolic.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,61 @@ def dereference_recursive(cls, repo: "Repo", ref_path: Union[PathLike, None]) ->
return hexsha
# END recursive dereferencing

@staticmethod
def _check_ref_name_valid(ref_path: PathLike) -> None:
# Based on the rules described in https://git-scm.com/docs/git-check-ref-format/#_description
previous: Union[str, None] = None
one_before_previous: Union[str, None] = None
for c in str(ref_path):
if c in " ~^:?*[\\":
raise ValueError(
f"Invalid reference '{ref_path}': references cannot contain spaces, tildes (~), carets (^),"
f" colons (:), question marks (?), asterisks (*), open brackets ([) or backslashes (\\)"
)
elif c == ".":
if previous is None or previous == "/":
raise ValueError(
f"Invalid reference '{ref_path}': references cannot start with a period (.) or contain '/.'"
)
elif previous == ".":
raise ValueError(f"Invalid reference '{ref_path}': references cannot contain '..'")
elif c == "/":
if previous == "/":
raise ValueError(f"Invalid reference '{ref_path}': references cannot contain '//'")
elif previous is None:
raise ValueError(
f"Invalid reference '{ref_path}': references cannot start with forward slashes '/'"
)
elif c == "{" and previous == "@":
raise ValueError(f"Invalid reference '{ref_path}': references cannot contain '@{{'")
elif ord(c) < 32 or ord(c) == 127:
raise ValueError(f"Invalid reference '{ref_path}': references cannot contain ASCII control characters")

one_before_previous = previous
previous = c

if previous == ".":
raise ValueError(f"Invalid reference '{ref_path}': references cannot end with a period (.)")
elif previous == "/":
raise ValueError(f"Invalid reference '{ref_path}': references cannot end with a forward slash (/)")
elif previous == "@" and one_before_previous is None:
raise ValueError(f"Invalid reference '{ref_path}': references cannot be '@'")
elif any([component.endswith(".lock") for component in str(ref_path).split("/")]):
raise ValueError(
f"Invalid reference '{ref_path}': references cannot have slash-separated components that end with"
f" '.lock'"
)

@classmethod
def _get_ref_info_helper(
cls, repo: "Repo", ref_path: Union[PathLike, None]
) -> Union[Tuple[str, None], Tuple[None, str]]:
"""Return: (str(sha), str(target_ref_path)) if available, the sha the file at
rela_path points to, or None. target_ref_path is the reference we
point to, or None"""
if ".." in str(ref_path):
raise ValueError(f"Invalid reference '{ref_path}'")
if ref_path:
cls._check_ref_name_valid(ref_path)

tokens: Union[None, List[str], Tuple[str, str]] = None
repodir = _git_dir(repo, ref_path)
try:
Expand Down
36 changes: 36 additions & 0 deletions test/test_refs.py
Original file line number Diff line number Diff line change
Expand Up @@ -631,3 +631,39 @@ def test_refs_outside_repo(self):
ref_file.flush()
ref_file_name = Path(ref_file.name).name
self.assertRaises(BadName, self.rorepo.commit, f"../../{ref_file_name}")

def test_validity_ref_names(self):
check_ref = SymbolicReference._check_ref_name_valid
# Based on the rules specified in https://git-scm.com/docs/git-check-ref-format/#_description
# Rule 1
self.assertRaises(ValueError, check_ref, ".ref/begins/with/dot")
self.assertRaises(ValueError, check_ref, "ref/component/.begins/with/dot")
self.assertRaises(ValueError, check_ref, "ref/ends/with/a.lock")
self.assertRaises(ValueError, check_ref, "ref/component/ends.lock/with/period_lock")
# Rule 2
check_ref("valid_one_level_refname")
# Rule 3
self.assertRaises(ValueError, check_ref, "ref/contains/../double/period")
# Rule 4
for c in " ~^:":
self.assertRaises(ValueError, check_ref, f"ref/contains/invalid{c}/character")
for code in range(0, 32):
self.assertRaises(ValueError, check_ref, f"ref/contains/invalid{chr(code)}/ASCII/control_character")
self.assertRaises(ValueError, check_ref, f"ref/contains/invalid{chr(127)}/ASCII/control_character")
# Rule 5
for c in "*?[":
self.assertRaises(ValueError, check_ref, f"ref/contains/invalid{c}/character")
# Rule 6
self.assertRaises(ValueError, check_ref, "/ref/begins/with/slash")
self.assertRaises(ValueError, check_ref, "ref/ends/with/slash/")
self.assertRaises(ValueError, check_ref, "ref/contains//double/slash/")
# Rule 7
self.assertRaises(ValueError, check_ref, "ref/ends/with/dot.")
# Rule 8
self.assertRaises(ValueError, check_ref, "ref/contains@{/at_brace")
# Rule 9
self.assertRaises(ValueError, check_ref, "@")
# Rule 10
self.assertRaises(ValueError, check_ref, "ref/contain\\s/backslash")
# Valid reference name should not raise
check_ref("valid/ref/name")

0 comments on commit e98f57b

Please sign in to comment.