From 674763717d18278b8b6bf7b4ee4fd110c8611468 Mon Sep 17 00:00:00 2001 From: Keerthana KT Date: Thu, 22 Oct 2026 16:44:22 +0530 Subject: [PATCH] fix(repo): split `git blame` porcelain output at line feeds only `Repo.blame` runs `git blame -p` and iterated over its stdout with `bytes.splitlines()`, which breaks at a lone carriage return as well as at a line feed. git ends every porcelain line with a line feed only, and the content lines in that output are the raw lines of the blamed file, so a CR inside a line of file content ended the line early in GitPython and the rest of it was parsed as porcelain headers. A file line such as ok\r<40 hex digits> 2 2 1\rauthor Forged Author\rauthor-mail \r...\r\tforged() made `Repo.blame` return an extra entry that attributes `forged()` to a commit id and author of the file author's choosing, while `git blame` attributes the whole line to the commit that really added it. Short of a forgery, any line with a lone CR was truncated at the CR and the rest of it dropped. Blamed file content is controlled by whoever wrote it, so a tool that uses `Repo.blame` to decide who wrote a line (code ownership, review routing, audit) could be shown attribution chosen by that author. Iterate over a `BytesIO` of the output instead, which splits at line feeds only and keeps the terminators, so CRLF lines and the binary-line handling are unchanged. `blame_incremental` does not print file content and is unaffected. Adds `test_blame_content_with_carriage_returns` to `test/test_repo.py`; it fails before this change and passes after. `test_blame_accepts_rev_opts` mocks `Git._call_process`, so it now returns `b""` instead of a `MagicMock` to the parser. `ruff check`, `ruff format --check` and `mypy` pass. In a full clone prepared with `init-tests-after-clone.sh`, the pytest suite passes except `test_diff_with_staged_file`, `test_empty_repo` and `test_rebasing`, which fail the same way without this change. --- git/repo/base.py | 7 +++++-- test/test_repo.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/git/repo/base.py b/git/repo/base.py index a4161d620..6bf885280 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -8,6 +8,7 @@ __all__ = ["Repo"] import gc +from io import BytesIO import logging import os import os.path as osp @@ -1358,8 +1359,10 @@ class InfoTD(TypedDict, total=False): info: InfoTD = {} - keepends = True - for line_bytes in data.splitlines(keepends): + # Git ends each porcelain line with a line feed only. bytes.splitlines() also + # splits at a lone carriage return, which a line of file content can contain, + # and would then parse the rest of that content line as blame headers. + for line_bytes in BytesIO(data): line_str = "" try: line_str = line_bytes.rstrip().decode(defenc) diff --git a/test/test_repo.py b/test/test_repo.py index 08cd4a1bf..8106a88ca 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -839,9 +839,44 @@ def test_blame_complex_revision(self, git): def test_blame_accepts_rev_opts(self, git): expected_args = ["blame", "HEAD", "-M", "-C", "-C", "--", "README.md"] boilerplate_kwargs = {"p": True, "stdout_as_string": False} + git.return_value = b"" self.rorepo.blame("HEAD", "README.md", rev_opts=["-M", "-C", "-C"]) git.assert_called_once_with(*expected_args, **boilerplate_kwargs) + @with_rw_directory + def test_blame_content_with_carriage_returns(self, rw_dir): + repo = Repo.init(rw_dir) + with repo.config_writer() as writer: + writer.set_value("user", "name", "GitPython Tests") + writer.set_value("user", "email", "gitpython@example.com") + + # A lone CR inside a line of content, followed by text shaped like blame + # porcelain headers for a forged commit and author. + forged = "\r".join( + [ + "ok", + "0" * 40 + " 2 2 1", + "author Forged Author", + "author-mail ", + "author-time 1000000000", + "committer Forged Author", + "committer-mail ", + "committer-time 1000000000", + "\tforged()", + ] + ) + tracked_file = Path(rw_dir) / "file.txt" + tracked_file.write_bytes(b"first\n" + forged.encode("ascii") + b"\n") + repo.index.add([str(tracked_file)]) + commit = repo.index.commit("init") + + blames = repo.blame("HEAD", "file.txt") + + self.assertEqual(len(blames), 1) + self.assertEqual(blames[0][0], commit) + self.assertEqual(blames[0][0].author.name, "GitPython Tests") + self.assertEqual(blames[0][1], ["first", forged]) + @with_rw_repo("HEAD", bare=False) def test_untracked_files(self, rwrepo): for run, repo_add in enumerate((rwrepo.index.add, rwrepo.git.add)):