diff --git a/VERSION b/VERSION index e5c812e68..c29b32b56 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.1.59 +3.1.61 diff --git a/doc/source/changes.rst b/doc/source/changes.rst index 1a1b8fa12..6b06dd5bf 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -2,6 +2,33 @@ Changelog ========= +3.1.61 +====== + +A fixup release to avoid accidental removal of public class regex on Actor. +It's now deprecated instead. + +If you can, also try and provide feedback on the upcoming v4 branch +https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. + +See the following for all changes. +https://github.com/gitpython-developers/GitPython/releases/tag/3.1.61 + +3.1.60 +====== + +Security fixes for + +* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-g5vv-9gxw-82hx +* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-whh4-5q6c-9v3x +* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-239g-whfq-7xj9 + +If you can, also try and provide feedback on the upcoming v4 branch +https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. + +See the following for all changes. +https://github.com/gitpython-developers/GitPython/releases/tag/3.1.60 + 3.1.59 ====== diff --git a/git/config.py b/git/config.py index 6f26e58fc..e7f64f7b5 100644 --- a/git/config.py +++ b/git/config.py @@ -462,7 +462,8 @@ def string_decode(v: str) -> str: v = v[:-1] # END cut trailing escapes to prevent decode error - return v.encode(defenc).decode("unicode_escape") + escapes = {"b": "\b", "n": "\n", "r": "\r", "t": "\t", '"': '"', "\\": "\\"} + return re.sub(r"\\(.)", lambda match: escapes.get(match.group(1), match.group(0)), v) # END string_decode @@ -517,10 +518,12 @@ def string_decode(v: str) -> str: # Opens quoting and does not close: appears to start multi-line quoting. is_multi_line = True optval = string_decode(optval[1:]) - elif optval.find("\\", 1, -1) == -1 and optval.find('"', 1, -1) == -1: - # Opens and closes quoting. Single line, and all we need is quote removal. - optval = optval[1:-1] - # TODO: Handle other quoted content, especially well-formed backslash escapes. + elif re.search(r'(?:^|[^\\])(?:\\\\)*"', optval[1:-1]): + # Preserve malformed values containing unescaped quotes. + pass + else: + # Opens and closes quoting. + optval = string_decode(optval[1:-1]) # Preserves multiple values for duplicate optnames. cursect.add(optname, optval) @@ -706,7 +709,7 @@ def write_section(name: str, section_dict: _OMD) -> None: for v in values: value = self._value_to_string(v) - if any(char in value for char in '\n\t\b\\"'): + if any(char in value for char in '\n\t\b\\"#;') or value[:1].isspace() or value[-1:].isspace(): value = value.replace("\\", "\\\\").replace('"', '\\"') value = '"%s\\\n"' % value.replace("\n", "\\n").replace("\t", "\\t").replace("\b", "\\b") fp.write(("\t%s = %s\n" % (key, value)).encode(defenc)) @@ -768,6 +771,20 @@ def write(self) -> None: return # END stop if we have include files + sections: List[_OMD] = [self._defaults] + section: _OMD + stored_section: _OMD + values: List[Any] + raw_value: Any + for _, stored_section in self._sections.items(): + sections.append(stored_section) + for section in sections: + for key, values in section.items_all(): + if key != "__name__": + for raw_value in values: + if "\r" in self._value_to_string(raw_value) or "\x00" in self._value_to_string(raw_value): + raise ValueError("Git config values must not contain CR or NUL") + fp = self._file_or_files # We have a physical file on disk, so get a lock. diff --git a/git/diff.py b/git/diff.py index d1963b84f..192c099d0 100644 --- a/git/diff.py +++ b/git/diff.py @@ -95,14 +95,35 @@ class DiffConstants(enum.Enum): :const:`git.INDEX` and :const:`Diffable.INDEX`, as well as :const:`Diffable.Index`. """ -_octal_byte_re = re.compile(rb"\\([0-9]{3})") - -def _octal_repl(matchobj: Match) -> bytes: - value = matchobj.group(1) - value = int(value, 8) - value = bytes(bytearray((value,))) - return value +def _unquote_path(path: bytes) -> bytes: + result = bytearray() + escapes = { + ord("a"): 7, + ord("b"): 8, + ord("f"): 12, + ord("n"): 10, + ord("r"): 13, + ord("t"): 9, + ord("v"): 11, + } + i = 0 + while i < len(path): + if path[i] != ord("\\") or i + 1 == len(path): + result.append(path[i]) + i += 1 + continue + if path[i + 1] in b"0123" and i + 3 < len(path) and all(c in b"01234567" for c in path[i + 2 : i + 4]): + result.append(int(path[i + 1 : i + 4], 8)) + i += 4 + continue + escaped = path[i + 1] + if escaped in escapes or escaped in b'\\"': + result.append(escapes.get(escaped, escaped)) + else: + result.extend(path[i : i + 2]) + i += 2 + return bytes(result) def decode_path(path: bytes, has_ab_prefix: bool = True) -> Optional[bytes]: @@ -110,9 +131,7 @@ def decode_path(path: bytes, has_ab_prefix: bool = True) -> Optional[bytes]: return None if path.startswith(b'"') and path.endswith(b'"'): - path = path[1:-1].replace(b"\\n", b"\n").replace(b"\\t", b"\t").replace(b'\\"', b'"').replace(b"\\\\", b"\\") - - path = _octal_byte_re.sub(_octal_repl, path) + path = _unquote_path(path[1:-1]) if has_ab_prefix: assert path.startswith(b"a/") or path.startswith(b"b/") @@ -220,8 +239,8 @@ def diff( to be read and diffed. :param allow_unsafe_options: - If ``True``, allow options such as ``--output`` and ``-O`` that can write to - or read from arbitrary filesystem paths. + If ``True``, allow options such as ``--output``, ``--no-index``, and ``-O`` + that can write to or read from arbitrary filesystem paths. :param kwargs: Additional arguments passed to :manpage:`git-diff(1)`, such as ``R=True`` to diff --git a/git/repo/base.py b/git/repo/base.py index d0ec00ec9..4ae48e17b 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -45,7 +45,6 @@ from .fun import ( find_submodule_git_dir, - find_worktree_git_dir, is_git_dir, rev_parse, touch, @@ -209,6 +208,8 @@ class Repo: ] unsafe_git_diff_options = unsafe_git_revision_options + [ + # Treats path operands as arbitrary filesystem paths. + "--no-index", # Reads caller-controlled order patterns from an arbitrary file. "-O", "--orderfile", @@ -232,6 +233,10 @@ def __init__( ) -> None: R"""Create a new :class:`Repo` instance. + .. note:: + Repositories using reftable may be opened, but GitPython's direct reference + access does not support reftable. + :param path: The path to either the worktree directory or the .git directory itself:: @@ -266,7 +271,11 @@ def __init__( :class:`Repo` """ - epath = path or os.getenv("GIT_DIR") + git_dir_env = os.getenv("GIT_DIR") + object_dir_env = os.getenv("GIT_OBJECT_DIRECTORY") + if object_dir_env is not None: + object_dir_env = osp.abspath(object_dir_env) + epath = path or git_dir_env if not epath: epath = os.getcwd() epath = os.fspath(epath) @@ -288,37 +297,48 @@ def __init__( raise NoSuchPathError(epath) # Walk up the path to find the `.git` dir. - curpath = epath - git_dir = None + curpath = os.fspath(epath) if epath is not None else "" + git_dir: Optional[str] = None + explicit_git_dir = not path and bool(git_dir_env) while curpath: # ABOUT osp.NORMPATH # It's important to normalize the paths, as submodules will otherwise # initialize their repo instances with paths that depend on path-portions # that will not exist after being removed. It's just cleaner. - if ( - osp.isfile(osp.join(curpath, "gitdir")) - and osp.isfile(osp.join(curpath, "commondir")) - and osp.isfile(osp.join(curpath, "HEAD")) - ): - git_dir = curpath - - if "GIT_WORK_TREE" in os.environ: - self._working_tree_dir = os.getenv("GIT_WORK_TREE") - else: - # Linked worktree administrative directories store the path to the - # worktree's .git file in their gitdir file (without "gitdir: " prefix). - with open(osp.join(git_dir, "gitdir")) as fp: - worktree_gitfile = fp.read().strip() + if not explicit_git_dir: + dotgit = osp.join(curpath, ".git") + try: + sm_gitpath = find_submodule_git_dir(dotgit) + except OSError: + break + if sm_gitpath is not None: + # Worktrees can use relative paths as of Git 2.48, so join to curpath. + git_dir = osp.normpath(osp.join(curpath, os.fspath(sm_gitpath))) + self._working_tree_dir = curpath + break + + # Like Git, do not fall back to a bare repository or parent directory when + # a non-directory .git entry exists but is not a valid gitfile. + if osp.exists(dotgit) and not osp.isdir(dotgit): + break - if not osp.isabs(worktree_gitfile): - worktree_gitfile = osp.normpath(osp.join(git_dir, worktree_gitfile)) + if is_git_dir(curpath): + git_dir = curpath + if osp.isfile(osp.join(curpath, "gitdir")) and osp.isfile(osp.join(curpath, "commondir")): + if "GIT_WORK_TREE" in os.environ: + self._working_tree_dir = os.getenv("GIT_WORK_TREE") + else: + # Linked worktree administrative directories store the path to + # the worktree's .git file in gitdir (without a "gitdir: " prefix). + with open(osp.join(git_dir, "gitdir")) as fp: + worktree_gitfile = fp.read().strip() - self._working_tree_dir = osp.dirname(worktree_gitfile) + if not osp.isabs(worktree_gitfile): + worktree_gitfile = osp.normpath(osp.join(git_dir, worktree_gitfile)) - break + self._working_tree_dir = osp.dirname(worktree_gitfile) + break - if is_git_dir(curpath): - git_dir = curpath # from man git-config : core.worktree # Set the path to the root of the working tree. If GIT_COMMON_DIR # environment variable is set, core.worktree is ignored and not used for @@ -338,22 +358,7 @@ def __init__( self._working_tree_dir = os.getenv("GIT_WORK_TREE") break - dotgit = osp.join(curpath, ".git") - sm_gitpath = find_submodule_git_dir(dotgit) - if sm_gitpath is not None: - git_dir = osp.normpath(sm_gitpath) - - sm_gitpath = find_submodule_git_dir(dotgit) - if sm_gitpath is None: - sm_gitpath = find_worktree_git_dir(dotgit) - - if sm_gitpath is not None: - # worktrees can use relative paths as of Git 2.48, so we join to curpath - git_dir = osp.normpath(osp.join(curpath, sm_gitpath)) - self._working_tree_dir = curpath - break - - if not search_parent_directories: + if explicit_git_dir or not search_parent_directories: break curpath, tail = osp.split(curpath) if not tail: @@ -364,6 +369,16 @@ def __init__( raise InvalidGitRepositoryError(epath) self.git_dir = git_dir + common_dir_env = os.getenv("GIT_COMMON_DIR") + if common_dir_env is not None: + self._common_dir = osp.abspath(common_dir_env) + else: + try: + common_dir = os.fsdecode((Path(self.git_dir) / "commondir").read_bytes()).rstrip("\r\n") + self._common_dir = osp.join(self.git_dir, common_dir) + except OSError: + self._common_dir = "" + self._bare = False try: self._bare = self.config_reader("repository").getboolean("core", "bare") @@ -371,12 +386,6 @@ def __init__( # Let's not assume the option exists, although it should. pass - try: - common_dir = (Path(self.git_dir) / "commondir").read_text().splitlines()[0].strip() - self._common_dir = osp.join(self.git_dir, common_dir) - except OSError: - self._common_dir = "" - # Adjust the working directory in case we are actually bare - we didn't know # that in the first place. if self._bare: @@ -385,9 +394,15 @@ def __init__( self.working_dir: PathLike = self._working_tree_dir or self.common_dir self.git = self.GitCommandWrapperType(self.working_dir) + if common_dir_env is not None: + self.git.update_environment(GIT_DIR=os.fspath(self.git_dir), GIT_COMMON_DIR=os.fspath(self.common_dir)) + elif git_dir_env is not None: + self.git.update_environment(GIT_DIR=os.fspath(self.git_dir)) + if object_dir_env is not None: + self.git.update_environment(GIT_OBJECT_DIRECTORY=object_dir_env) # Special handling, in special times. - rootpath = osp.join(self.common_dir, "objects") + rootpath = object_dir_env if object_dir_env is not None else osp.join(self.common_dir, "objects") if issubclass(odbt, GitCmdObjectDB): self.odb = odbt(rootpath, self.git) else: @@ -988,7 +1003,7 @@ def _get_alternates(self) -> List[str]: :return: List of strings being pathnames of alternates """ - alternates_path = osp.join(self.common_dir, "objects", "info", "alternates") + alternates_path = osp.join(self.odb.root_path(), "info", "alternates") if osp.exists(alternates_path): with open(alternates_path, "rb") as f: @@ -1009,7 +1024,7 @@ def _set_alternates(self, alts: List[str]) -> None: The method does not check for the existence of the paths in `alts`, as the caller is responsible. """ - alternates_path = osp.join(self.common_dir, "objects", "info", "alternates") + alternates_path = osp.join(self.odb.root_path(), "info", "alternates") if not alts: if osp.isfile(alternates_path): os.remove(alternates_path) diff --git a/git/repo/fun.py b/git/repo/fun.py index 66e7eba69..eb0d8075a 100644 --- a/git/repo/fun.py +++ b/git/repo/fun.py @@ -60,22 +60,61 @@ def touch(filename: str) -> str: def is_git_dir(d: PathLike) -> bool: """This is taken from the git setup.c:is_git_directory function. + .. note:: + This function recognizes repositories using reftable through their + compatibility files, but GitPython's direct reference access does not support + reftable. + :raise git.exc.WorkTreeRepositoryUnsupported: If it sees a worktree directory. It's quite hacky to do that here, but at least clearly indicates that we don't support it. There is the unlikely danger to throw if we see directories which just look like a worktree dir, but are none. """ if osp.isdir(d): - if (osp.isdir(osp.join(d, "objects")) or "GIT_OBJECT_DIRECTORY" in os.environ) and osp.isdir( - osp.join(d, "refs") - ): - headref = osp.join(d, "HEAD") - return osp.isfile(headref) or (osp.islink(headref) and os.readlink(headref).startswith("refs")) - elif ( - osp.isfile(osp.join(d, "gitdir")) - and osp.isfile(osp.join(d, "commondir")) - and osp.isfile(osp.join(d, "gitfile")) - ): + headref = osp.join(d, "HEAD") + if osp.islink(headref): + try: + valid_head = os.readlink(headref).startswith("refs/") + except OSError: + valid_head = False + else: + try: + with open(headref, "rb") as fp: + head = fp.read(256) + except OSError: + valid_head = False + else: + valid_head = (head.startswith(b"ref:") and head[4:].lstrip().startswith(b"refs/")) or bool( + re.match(rb"(?:[0-9A-Fa-f]{64}|[0-9A-Fa-f]{40})", head) + ) + + common_dir = os.getenv("GIT_COMMON_DIR") + if common_dir == "": + return False + if common_dir is None: + common_dir_file = Path(d) / "commondir" + try: + common_dir = os.fsdecode(common_dir_file.read_bytes()).rstrip("\r\n") + except FileNotFoundError: + if osp.lexists(common_dir_file): + return False + common_dir = os.fspath(d) + except (OSError, UnicodeError): + return False + else: + if not common_dir: + return False + try: + common_dir = osp.realpath(osp.join(d, common_dir)) + except (OSError, ValueError): + return False + + object_dir = os.getenv("GIT_OBJECT_DIRECTORY") + if object_dir is None: + object_dir = osp.join(common_dir, "objects") + if valid_head and osp.isdir(object_dir) and osp.isdir(osp.join(common_dir, "refs")): + return True + if osp.isfile(osp.join(d, "gitdir")) and osp.isfile(osp.join(d, "commondir")) and osp.isfile(headref): raise WorkTreeRepositoryUnsupported(d) return False @@ -84,19 +123,20 @@ def find_worktree_git_dir(dotgit: PathLike) -> Optional[str]: """Search for a gitdir for this worktree.""" try: statbuf = os.stat(dotgit) - except OSError: + except (FileNotFoundError, NotADirectoryError): return None - if not stat.S_ISREG(statbuf.st_mode): + if not stat.S_ISREG(statbuf.st_mode) or statbuf.st_size > (1 << 20): return None try: - lines = Path(dotgit).read_text().splitlines() - for key, value in [line.strip().split(": ") for line in lines]: - if key == "gitdir": - return value - except ValueError: - pass - return None + with open(dotgit, "rb") as fp: + content_bytes = fp.read(statbuf.st_size) + if len(content_bytes) != statbuf.st_size: + return None + content = os.fsdecode(content_bytes).rstrip("\r\n") + except (OSError, UnicodeError): + return None + return content[8:] if len(content) >= 9 and content.startswith("gitdir: ") else None def find_submodule_git_dir(d: PathLike) -> Optional[PathLike]: @@ -104,26 +144,18 @@ def find_submodule_git_dir(d: PathLike) -> Optional[PathLike]: if is_git_dir(d): return d - try: - with open(d) as fp: - content = fp.read().rstrip() - except IOError: - # It's probably not a file. - pass - else: - if content.startswith("gitdir: "): - path = content[8:] - - if Git.is_cygwin(): - # Cygwin creates submodules prefixed with `/cygdrive/...`. - # Cygwin git understands Cygwin paths much better than Windows ones. - # Also the Cygwin tests are assuming Cygwin paths. - path = cygpath(path) - if not osp.isabs(path): - path = osp.normpath(osp.join(osp.dirname(d), path)) - return find_submodule_git_dir(path) - # END handle exception - return None + path = find_worktree_git_dir(d) + if path is None: + return None + + if Git.is_cygwin(): + # Cygwin creates submodules prefixed with `/cygdrive/...`. + # Cygwin git understands Cygwin paths much better than Windows ones. + # Also the Cygwin tests are assuming Cygwin paths. + path = cygpath(path) + if not osp.isabs(path): + path = osp.normpath(osp.join(osp.dirname(d), path)) + return path if is_git_dir(path) else None def short_to_long(odb: "GitCmdObjectDB", hexsha: str) -> Optional[bytes]: diff --git a/git/util.py b/git/util.py index 02f57c132..a80e667c7 100644 --- a/git/util.py +++ b/git/util.py @@ -80,6 +80,7 @@ Sequence, Tuple, TYPE_CHECKING, + Type, TypeVar, Union, cast, @@ -108,6 +109,7 @@ T_IterableObj = TypeVar("T_IterableObj", bound=Union["IterableObj", "Has_id_attribute"], covariant=True) # So IterableList[Head] is subtype of IterableList[IterableObj]. +T_Actor = TypeVar("T_Actor", bound="Actor") _logger = logging.getLogger(__name__) @@ -853,14 +855,26 @@ def update(self, *args: Any, **kwargs: Any) -> None: self._callable(*args, **kwargs) +class _DeprecatedActorNameEmailRegex: + _pattern = re.compile(r"(.*) <(.*?)>") + + def __get__(self, _instance: Any, _owner: Any) -> Pattern[str]: + warnings.warn( + "Actor.name_email_regex is deprecated and will be removed in GitPython 4.0.0 because searching long " + "malformed strings with it can take quadratic time. Use Actor.from_string() to parse actor identities, " + "or Actor(name, email) when the fields are already separate.", + DeprecationWarning, + stacklevel=2, + ) + return self._pattern + + class Actor: """Actors hold information about a person acting on the repository. They can be committers and authors or anything with a name and an email as mentioned in the git log entries.""" - # PRECOMPILED REGEX - name_only_regex = re.compile(r"<(.*)>") - name_email_regex = re.compile(r"(.*) <(.*?)>") + name_email_regex = _DeprecatedActorNameEmailRegex() # ENVIRONMENT VARIABLES # These are read when creating new commits. @@ -895,7 +909,7 @@ def __repr__(self) -> str: return '">' % (self.name, self.email) @classmethod - def _from_string(cls, string: str) -> "Actor": + def from_string(cls: Type[T_Actor], string: str) -> T_Actor: """Create an :class:`Actor` from a string. :param string: @@ -906,18 +920,16 @@ def _from_string(cls, string: str) -> "Actor": :return: :class:`Actor` """ - m = cls.name_email_regex.search(string) - if m: - name, email = m.groups() - return Actor(name, email) - else: - m = cls.name_only_regex.search(string) - if m: - return Actor(m.group(1), None) - # Assume the best and use the whole string as name. - return Actor(string, None) - # END special case name - # END handle name/email matching + line = string.partition("\n")[0] + left_bracket = line.find("<") + right_bracket = line.find(">", left_bracket + 1) + if left_bracket >= 0 and right_bracket >= 0: + return cls(line[:left_bracket].rstrip(), line[left_bracket + 1 : right_bracket]) + + # Assume the best and use the whole string as name. + return cls(string, None) + + _from_string = from_string @classmethod def _main_actor( diff --git a/test/lib/helper.py b/test/lib/helper.py index 58923eaef..4135fe5dd 100644 --- a/test/lib/helper.py +++ b/test/lib/helper.py @@ -32,6 +32,7 @@ import os import os.path as osp from stat import S_ISLNK, ST_MODE +import socket import subprocess import sys import tempfile @@ -218,8 +219,15 @@ def git_daemon_launched(base_path, ip, port): base_path=base_path, as_process=True, ) - # Yes, I know... fortunately, this is always going to work if sleep time is just large enough. - time.sleep(1.0 if sys.platform == "win32" else 0.5) + + # Wait until git daemon listens for connections. + for _attempt in range(1, 30): + try: + socket.create_connection((ip, port), timeout=30).close() + break + except ConnectionRefusedError: + time.sleep(0.5) + except Exception as ex: msg = textwrap.dedent( """ diff --git a/test/test_actor.py b/test/test_actor.py index 5e6635709..68afb80d3 100644 --- a/test/test_actor.py +++ b/test/test_actor.py @@ -3,14 +3,15 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ -from git import Actor +from unittest import mock +from git import Actor from test.lib import TestBase class TestActor(TestBase): def test_from_string_should_separate_name_and_email(self): - a = Actor._from_string("Michael Trier ") + a = Actor.from_string("Michael Trier ") self.assertEqual("Michael Trier", a.name) self.assertEqual("mtrier@example.com", a.email) @@ -23,14 +24,50 @@ def test_from_string_should_separate_name_and_email(self): assert len(m) == 1 def test_from_string_should_handle_just_name(self): - a = Actor._from_string("Michael Trier") + a = Actor.from_string("Michael Trier") self.assertEqual("Michael Trier", a.name) self.assertEqual(None, a.email) + def test_from_string_constructs_subclass(self): + class DerivedActor(Actor): + pass + + self.assertIsInstance(DerivedActor.from_string("name "), DerivedActor) + + def test_from_string_handles_unterminated_email_without_regex_backtracking(self): + value = "A" * 20_000 + " ") + + message = str(context.warning) + self.assertIn("Actor.from_string()", message) + self.assertIn("Actor(name, email)", message) + assert match is not None + self.assertEqual(match.groups(), ("Michael Trier", "mtrier@example.com")) + + def test_from_string_does_not_parse_across_lines(self): + self.assertEqual(Actor.from_string("x \n y "), Actor("x", "a")) + + def test_from_string_uses_git_delimiters(self): + for value, expected in ( + ("Name ", Actor("Name", "e>", Actor("Name", "email")), + ("Name", Actor("Name", "email")), + (" <>", Actor("", "")), + ("Name ", Actor("Name email>", None)), + ): + self.assertEqual(Actor.from_string(value), expected) + def test_should_display_representation(self): - a = Actor._from_string("Michael Trier ") + a = Actor.from_string("Michael Trier ") self.assertEqual('">', repr(a)) def test_str_should_alias_name(self): - a = Actor._from_string("Michael Trier ") + a = Actor.from_string("Michael Trier ") self.assertEqual(a.name, str(a)) diff --git a/test/test_config.py b/test/test_config.py index d664fdb6f..28bb12043 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -14,6 +14,7 @@ import pytest from git import GitConfigParser +from git.compat import defenc from git.config import _OMD, cp from git.util import cwd, rmfile from test.lib import SkipTest, TestCase, fixture_path, with_rw_directory @@ -170,7 +171,16 @@ def test_rewriting_multiline_value_does_not_create_option(self, rw_dir): @with_rw_directory def test_writer_escapes_special_characters_without_newline(self, rw_dir): config_path = osp.join(rw_dir, "config") - values = {"tab": "\tvalue\t", "backspace": "a\bb", "quote": 'a"b', "backslash": "a\\qb"} + values = { + "tab": "\tvalue\t", + "backspace": "a\bb", + "quote": 'a"b', + "backslash": "a\\qb", + "hash": "value#fragment", + "semicolon": "value;fragment", + "leading": " value", + "trailing": "value ", + } with GitConfigParser(config_path, read_only=False) as git_config: for key, value in values.items(): @@ -185,11 +195,72 @@ def test_writer_escapes_special_characters_without_newline(self, rw_dir): stdout=subprocess.PIPE, check=True, ).stdout, - value.encode() + b"\n", + value.encode(defenc) + b"\n", ) with open(config_path, "rb") as config_file: self.assertNotIn(b"\x08", config_file.read()) + @with_rw_directory + def test_writer_preserves_escaped_and_non_ascii_values_safely(self, rw_dir): + config_path = osp.join(rw_dir, "config") + with open(config_path, "wb") as config_file: + config_file.write( + ( + '[section]\nnewline = "first\\nsecond"\nquote = "a\\"b"\nbackslash = "a\\\\b"\n' + 'unicode = "café\\\\path"\n' + ).encode(defenc) + ) + + with GitConfigParser(config_path, read_only=False) as config: + config.set_value("unrelated", "key", "value") + + expected = { + "newline": "first\nsecond", + "quote": 'a"b', + "backslash": "a\\b", + "unicode": "café\\path", + } + with GitConfigParser(config_path, read_only=True) as config: + for key, value in expected.items(): + self.assertEqual( + config.get_value("section", key), + value, + "GitPython should preserve values when rewriting unrelated entries", + ) + self.assertEqual( + subprocess.run( + ["git", "config", "--file", config_path, "--get", "section.%s" % key], + stdout=subprocess.PIPE, + check=True, + ).stdout, + value.encode(defenc) + b"\n", + "git should read rewritten values with the same semantics", + ) + + with open(config_path, "rb") as config_file: + contents = config_file.read() + self.assertNotIn(b"\r", contents, "the writer should never emit carriage returns") + self.assertNotIn(b"\x00", contents, "the writer should never emit NUL bytes") + + for name, value in (("return", b"first\\rsecond"), ("nul", b"first\x00second")): + unsafe_path = osp.join(rw_dir, "%s-config" % name) + unsafe_contents = b'[section]\nvalue = "' + value + b'"\n' + with open(unsafe_path, "wb") as config_file: + config_file.write(unsafe_contents) + with self.assertRaisesRegex( + ValueError, + "CR or NUL", + msg="unsafe existing values should abort rewrites", + ): + with GitConfigParser(unsafe_path, read_only=False) as config: + config.set_value("unrelated", "key", "value") + with open(unsafe_path, "rb") as config_file: + self.assertEqual( + config_file.read(), + unsafe_contents, + "rejected rewrites should leave the original file unchanged", + ) + @with_rw_directory def test_set_value_rejects_config_injection(self, rw_dir): config_path = osp.join(rw_dir, "config") @@ -745,15 +816,14 @@ def test_config_with_quotes_with_whitespace_outside_value(self): self.assertEqual(cr.get("init", "defaultBranch"), "trunk") def test_config_with_quotes_containing_escapes(self): - """For now just suppress quote removal. But it would be good to interpret most of these.""" + """Interpret Git's quoted escapes without changing malformed values.""" cr = GitConfigParser(fixture_path("git_config_with_quotes_escapes"), read_only=True) - # These can eventually be supported by substituting the represented character. - self.assertEqual(cr.get("custom", "hasnewline"), R'"first\nsecond"') - self.assertEqual(cr.get("custom", "hasbackslash"), R'"foo\\bar"') - self.assertEqual(cr.get("custom", "hasquote"), R'"ab\"cd"') - self.assertEqual(cr.get("custom", "hastrailingbackslash"), R'"word\\"') - self.assertEqual(cr.get("custom", "hasunrecognized"), R'"p\qrs"') + self.assertEqual(cr.get("custom", "hasnewline"), "first\nsecond") + self.assertEqual(cr.get("custom", "hasbackslash"), R"foo\bar") + self.assertEqual(cr.get("custom", "hasquote"), 'ab"cd') + self.assertEqual(cr.get("custom", "hastrailingbackslash"), "word\\") + self.assertEqual(cr.get("custom", "hasunrecognized"), R"p\qrs") # It is less obvious whether and what to eventually do with this. self.assertEqual(cr.get("custom", "hasunescapedquotes"), '"ab"cd"e"') diff --git a/test/test_diff.py b/test/test_diff.py index d5e14f3de..9cfbffd17 100644 --- a/test/test_diff.py +++ b/test/test_diff.py @@ -14,6 +14,7 @@ from git import NULL_TREE, Diff, DiffIndex, Diffable, GitCommandError, Repo, Submodule from git.cmd import Git +from git.diff import decode_path from git.exc import UnsafeOptionError from test.lib import StringProcessAdapter, TestBase, fixture, with_rw_directory @@ -324,6 +325,11 @@ def test_diff_patch_format(self): Diff._index_from_patch_format(self.rorepo, diff_proc) # END for each fixture + def test_decode_path_distinguishes_escaped_backslashes_from_octal_bytes(self): + self.assertEqual(decode_path(b'"foo\\\\899bar"', False), b"foo\\899bar") + self.assertEqual(decode_path(b'"foo\\\\123bar"', False), b"foo\\123bar") + self.assertEqual(decode_path(b'"foo\\123bar"', False), b"fooSbar") + def test_diff_with_spaces(self): data = StringProcessAdapter(fixture("diff_file_with_spaces")) diff_index = Diff._index_from_patch_format(self.rorepo, data) @@ -406,6 +412,17 @@ def test_diff_rejects_unsafe_output_options(self): commit.diff(output=allowed_target, allow_unsafe_options=True) self.assertTrue(osp.isfile(allowed_target)) + def test_diff_rejects_no_index(self): + calls = ( + lambda: self.rorepo.head.commit.diff(no_index=True), + lambda: self.rorepo.head.commit.diff(other="--no-index"), + lambda: self.rorepo.index.diff(None, no_index=True), + lambda: self.rorepo.index.diff("--no-index"), + ) + for call in calls: + with self.assertRaises(UnsafeOptionError): + call() + def test_diff_interface(self): """Test a few variations of the main diff routine.""" assertion_map = {} diff --git a/test/test_repo.py b/test/test_repo.py index 1dfec951a..12e572f52 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -40,7 +40,8 @@ from git.exc import UnsafeOptionError from git.exc import UnsafeProtocolError from git.exc import BadObject -from git.repo.fun import touch +from git.exc import WorkTreeRepositoryUnsupported +from git.repo.fun import find_worktree_git_dir, touch from git.util import bin_to_hex, cwd, cygpath, join_path_native, rmfile, rmtree from test.lib import TestBase, fixture, requires_symlinks, with_rw_directory, with_rw_repo, PathLikeMock @@ -122,6 +123,178 @@ def test_new_should_raise_on_non_existent_path(self): nonexistent = osp.join(tdir, "foobar") self.assertRaises(NoSuchPathError, Repo, nonexistent) + def test_repo_discovery_prefers_dotgit(self): + layouts = { + "linked-worktree": { + "gitdir": ".git\n", + "commondir": ".git\n", + "HEAD": "ref: refs/heads/main\n", + }, + "bare": {"objects": None, "refs": None, "HEAD": "ref: refs/heads/main\n"}, + } + + with tempfile.TemporaryDirectory() as tdir: + for name, entries in layouts.items(): + path = Path(tdir) / name + Repo.init(path).close() + for entry, contents in entries.items(): + item = path / entry + if contents is None: + item.mkdir() + else: + item.write_text(contents) + + with self.subTest(layout=name): + expected_git_dir = Git(path).rev_parse("--absolute-git-dir") + assert osp.samefile(Repo(path).git_dir, expected_git_dir) + + def test_repo_discovery_honors_explicit_git_dir(self): + with tempfile.TemporaryDirectory() as tdir: + git_dir = Path(tdir) / "repo.git" + Repo.init(git_dir, bare=True).close() + Repo.init(git_dir / ".git", bare=True).close() + + with mock.patch.dict(os.environ, {"GIT_DIR": os.fspath(git_dir)}): + for path in (None, ""): + with Repo(path) as repo, self.subTest(path=path): + assert osp.samefile(repo.git_dir, git_dir) + + worktree = Path(tdir) / "worktree" + Repo.init(worktree).close() + with cwd(worktree), mock.patch.dict(os.environ, {"GIT_DIR": ""}): + with Repo() as repo: + assert osp.samefile(repo.git_dir, worktree / ".git") + + def test_repo_discovery_rejects_invalid_metadata(self): + with tempfile.TemporaryDirectory() as tdir: + path = Path(tdir) + (path / "objects").mkdir() + (path / "refs").mkdir() + (path / "HEAD").write_text("not a ref") + + with self.subTest(metadata="HEAD"): + self.assertRaises(InvalidGitRepositoryError, Repo, path) + + (path / "HEAD").write_text("ref: refs/heads/main\n") + + for contents in (b"", b"\xff"): + (path / "commondir").write_bytes(contents) + with cwd(path), self.subTest(metadata="commondir", contents=contents): + self.assertRaises(InvalidGitRepositoryError, Repo, path) + + (path / "gitdir").write_text("../worktree/.git\n") + (path / "commondir").write_text("missing\n") + with self.subTest(metadata="linked-worktree"): + self.assertRaises(WorkTreeRepositoryUnsupported, Repo, path) + + (path / "gitdir").unlink() + (path / "commondir").unlink() + for variable in ("GIT_COMMON_DIR", "GIT_OBJECT_DIRECTORY"): + with mock.patch.dict(os.environ, {variable: ""}), self.subTest(metadata=variable): + self.assertRaises(InvalidGitRepositoryError, Repo, path) + + for contents in (b"not a gitfile", b"gitdir: \n", b"gitdir: .git\n", b"\xff"): + (path / ".git").write_bytes(contents) + with self.subTest(metadata=".git", contents=contents): + self.assertRaises(InvalidGitRepositoryError, Repo, path) + + @requires_symlinks + def test_repo_discovery_rejects_dangling_commondir(self): + with tempfile.TemporaryDirectory() as tdir: + path = Path(tdir) + (path / "objects").mkdir() + (path / "refs").mkdir() + (path / "HEAD").write_text("ref: refs/heads/main\n") + (path / "commondir").symlink_to("missing") + + self.assertRaises(InvalidGitRepositoryError, Repo, path) + + @requires_symlinks + def test_repo_discovery_rejects_dotgit_stat_errors(self): + with tempfile.TemporaryDirectory() as tdir: + path = Path(tdir) + Repo.init(path).close() + child = path / "child" + child.mkdir() + (child / ".git").symlink_to(".git") + + self.assertRaises(InvalidGitRepositoryError, Repo, child, search_parent_directories=True) + + def test_gitfile_read_is_bounded(self): + with tempfile.TemporaryDirectory() as tdir: + dotgit = Path(tdir) / ".git" + content = b"gitdir: target\n" + dotgit.write_bytes(content) + reader = mock.mock_open(read_data=b"") + + with mock.patch("builtins.open", reader): + assert find_worktree_git_dir(dotgit) is None + + reader().read.assert_called_once_with(len(content)) + + def test_repo_discovery_uses_storage_environment(self): + with tempfile.TemporaryDirectory() as tdir: + git_dir = Path(tdir) / "git" + common_dir = Path(tdir) / "common" + git_dir.mkdir() + common_dir.mkdir() + (git_dir / "HEAD").write_text("ref: refs/heads/main\n") + (common_dir / "objects").mkdir() + (common_dir / "refs").mkdir() + (common_dir / "config").write_text("[core]\n\tbare = true\n") + + with cwd(tdir): + with mock.patch.dict(os.environ, {"GIT_DIR": "git", "GIT_COMMON_DIR": "common"}): + repo = Repo() + + assert osp.samefile(repo.common_dir, common_dir) + assert osp.samefile(repo.odb.root_path(), common_dir / "objects") + assert repo.bare + assert osp.samefile(repo.git.rev_parse("--absolute-git-dir"), git_dir) + assert osp.samefile(repo.git.rev_parse("--git-common-dir"), common_dir) + + (git_dir / "commondir").write_text("../common\n") + environment = dict(os.environ) + environment["GIT_DIR"] = "git" + environment.pop("GIT_COMMON_DIR", None) + with cwd(tdir), mock.patch.dict(os.environ, environment, clear=True): + repo = Repo() + + assert osp.samefile(repo.git.rev_parse("--absolute-git-dir"), git_dir) + + if sys.platform.startswith("linux"): + byte_common_dir = Path(tdir) / os.fsdecode(b"common-\xff") + byte_common_dir.mkdir() + (byte_common_dir / "objects").mkdir() + (byte_common_dir / "refs").mkdir() + (git_dir / "commondir").write_bytes(b"../common-\xff\n") + + assert osp.samefile(Repo(git_dir).common_dir, byte_common_dir) + + @with_rw_directory + def test_repo_discovery_preserves_object_directory(self, tdir): + git_dir = Path(tdir) / "git" + payload = b"custom object database" + payload_file = Path(tdir) / "payload" + payload_file.write_bytes(payload) + + source_repo = Repo.init(git_dir, bare=True) + blob_hexsha = source_repo.git.hash_object("-w", payload_file) + source_repo.close() + object_dir = Path(tdir) / "objects" + (git_dir / "objects").rename(object_dir) + + with cwd(tdir), mock.patch.dict(os.environ, {"GIT_DIR": "git", "GIT_OBJECT_DIRECTORY": "objects"}): + repo = Repo(odbt=GitDB) + + with repo: + assert osp.samefile(repo.odb.root_path(), object_dir) + assert repo.odb.has_object(bytes.fromhex(blob_hexsha)) + assert repo.git.cat_file("blob", blob_hexsha) == payload.decode() + repo.alternates = ["other/location"] + assert repo.alternates == ["other/location"] + assert (object_dir / "info" / "alternates").is_file() + @with_rw_repo("0.3.2.1") def test_repo_creation_from_different_paths(self, rw_repo): r_from_gitdir = Repo(rw_repo.git_dir) @@ -389,6 +562,7 @@ def test_alternates_use_common_dir(self, rw_dir): os.makedirs(osp.join(common_dir, "objects", "info")) os.makedirs(osp.join(git_dir, "objects", "info")) repo = mock.Mock(common_dir=common_dir, git_dir=git_dir) + repo.odb.root_path.return_value = osp.join(common_dir, "objects") alts = ["other/location", "this/location"] Repo._set_alternates(repo, alts) @@ -1147,6 +1321,30 @@ def test_empty_repo_reftable_active_branch(self, rw_dir): lambda: repo.active_branch, ) + @with_rw_directory + def test_reftable_repo_opens_but_direct_refs_are_unsupported(self, rw_dir): + git = Git(rw_dir) + try: + git.init(ref_format="reftable") + except GitCommandError as err: + if err.status == 129: + pytest.skip("git init --ref-format is not supported by this git version") + raise + + git.update_environment( + GIT_AUTHOR_NAME="Test Author", + GIT_AUTHOR_EMAIL="author@example.com", + GIT_COMMITTER_NAME="Test Committer", + GIT_COMMITTER_EMAIL="committer@example.com", + ) + git.commit(allow_empty=True, message="initial commit") + expected_head = git.rev_parse("HEAD") + + repo = Repo(rw_dir) + assert repo.git.rev_parse("HEAD") == expected_head + assert repo.head.reference.name == ".invalid" + assert not repo.heads + @with_rw_directory def test_active_branch_raises_type_error_when_head_is_detached(self, rw_dir): repo = Repo.init(rw_dir)