Skip to content

Commit 6c99e04

Browse files
committed
fix: send one object name or path per git stdin record
`Git._prepare_ref` terminates an object name with a line feed and writes it to the persistent `git cat-file --batch-check` or `--batch` process, and `__get_object_header` reads exactly one response line back. A name that itself contains a line feed is therefore two requests against one response read, and the stream is left one response behind for the lifetime of the `Git` instance, so every later lookup is answered with the header of an object it did not ask for. `Repo.is_valid_object` hands its argument straight to `partial_to_complete_sha_hex`, so a single call with an untrusted revision string is enough to get there. After `repo.is_valid_object("HEAD\nHEAD")`, `repo.commit(<sha>)` returns a different commit than the one named and `Repo.odb.info` reports another object's type and size, with nothing to indicate it. On the `--batch` stream the size from the mismatched header also misaligns the content reads, so object data is read from the wrong offsets and a tree read right afterwards fails with `Invalid tree entry mode`. Git has no way to express an object name containing a line feed on that stdin, so `_prepare_ref` now refuses one and keeps the single trailing line feed that is the request terminator. `Repo.is_valid_object` reports such a name as invalid, which it is, and `short_to_long` keeps turning it into `BadName`. `IndexFile._write_path_to_stdin` writes paths to `git checkout-index --stdin` the same way, and unlike an object name a path can legitimately contain a line feed. With `dir/x\nkeep.txt` in the index, `index.checkout(paths=["dir"], force=True)` had Git read two paths, so `keep.txt` was checked out over the local copy while the requested file was never written. Git also unquotes a line-feed separated path that begins with a double quote, which made a file named `"q"` impossible to check out. That call site now runs with `-z` and NUL-terminated paths, which Git does not unquote. Adds regression tests in `test/test_git.py` and `test/test_index.py`, both of which fail before this change. The rest of the suite is unaffected on Python 3.11 on macOS, apart from eight failures that predate the change in `test_commit.py` and `test_submodule.py`; `ruff`, `mypy`, `basedpyright --warnings` and the docs build are clean.
1 parent f9e74ab commit 6c99e04

4 files changed

Lines changed: 49 additions & 8 deletions

File tree

‎git/cmd.py‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1940,9 +1940,15 @@ def _prepare_ref(self, ref: object) -> bytes:
19401940
else:
19411941
refstr = ref
19421942

1943-
if not refstr.endswith("\n"):
1944-
refstr += "\n"
1945-
return refstr.encode(defenc)
1943+
# A line feed terminates a request, so one object name must be one line. An
1944+
# embedded one would queue a second request on the persistent command while
1945+
# only one response line is read back, leaving every later call one response
1946+
# behind, answered with the header of an object it did not ask for.
1947+
if refstr.endswith("\n"):
1948+
refstr = refstr[:-1]
1949+
if "\n" in refstr:
1950+
raise ValueError("Object name %r contains a line feed" % refstr)
1951+
return (refstr + "\n").encode(defenc)
19461952

19471953
def _get_persistent_cmd(self, attr_name: str, cmd_name: str, *args: Any, **kwargs: Any) -> "Git.AutoInterrupt":
19481954
cur_val = getattr(self, attr_name)

‎git/index/base.py‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -525,17 +525,17 @@ def _write_path_to_stdin(
525525
the piped-in files are processed anyway and just in time.
526526
527527
:note:
528-
Newlines are essential here, git's behaviour is somewhat inconsistent on
529-
this depending on the version, hence we try our best to deal with newlines
530-
carefully. Usually the last newline will not be sent, instead we will close
531-
stdin to break the pipe.
528+
Paths are NUL-terminated, so the command has to run with ``-z``. A path can
529+
contain a line feed, and with line-feed separation git would read such a
530+
path as two paths and act on files that were never passed. git also unquotes
531+
a line-feed separated path that begins with a double quote.
532532
"""
533533
fprogress(filepath, False, item)
534534
rval: Union[None, str] = None
535535

536536
if proc.stdin is not None:
537537
try:
538-
proc.stdin.write(("%s\n" % filepath).encode(defenc))
538+
proc.stdin.write(("%s\0" % filepath).encode(defenc))
539539
except OSError as e:
540540
# Pipe broke, usually because some error happened.
541541
raise fmakeexc() from e
@@ -1452,6 +1452,7 @@ def handle_stderr(proc: "Popen[bytes]", iter_checked_out_files: Iterable[PathLik
14521452
# initialization.
14531453
self.entries # noqa: B018
14541454

1455+
args.append("-z")
14551456
args.append("--stdin")
14561457
kwargs["as_process"] = True
14571458
kwargs["istream"] = subprocess.PIPE

‎test/test_git.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,19 @@ def test_persistent_cat_file_command(self):
593593
self.assertEqual(typename, typename_two)
594594
self.assertEqual(size, size_two)
595595

596+
def test_object_header_rejects_an_embedded_line_feed(self):
597+
hexsha = "b2339455342180c7cc1e9bba3e9f181f7baa5167"
598+
git = Git(self.rorepo.working_dir)
599+
header = git.get_object_header(hexsha)
600+
601+
# A single trailing line feed is the request terminator, not a second request.
602+
self.assertEqual(git.get_object_header(hexsha + "\n"), header)
603+
604+
self.assertRaises(ValueError, git.get_object_header, "HEAD\nHEAD")
605+
606+
# The persistent command is still in step, so this is not HEAD's header.
607+
self.assertEqual(git.get_object_header(hexsha), header)
608+
596609
def test_version_info(self):
597610
"""The version_info attribute is a tuple of up to four ints."""
598611
v = self.git.version_info

‎test/test_index.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1878,6 +1878,27 @@ def test_checkout_pathlike(self, tmp_path, path_type, absolute, directory, conta
18781878
for name, data in files.items():
18791879
assert (tmp_path / name).read_bytes() == data
18801880

1881+
@pytest.mark.skipif(os.name == "nt", reason="Line feeds and quotes are not valid Windows filenames")
1882+
def test_checkout_sends_each_path_as_one_record(self, tmp_path):
1883+
with Repo.init(tmp_path) as repo:
1884+
nested = tmp_path / "nested"
1885+
nested.mkdir()
1886+
(nested / "first\noutside").write_bytes(b"nested")
1887+
(tmp_path / "outside").write_bytes(b"committed")
1888+
(tmp_path / '"quoted"').write_bytes(b"quoted")
1889+
repo.index.add(["nested", "outside", '"quoted"'])
1890+
1891+
(nested / "first\noutside").unlink()
1892+
(tmp_path / '"quoted"').unlink()
1893+
(tmp_path / "outside").write_bytes(b"local")
1894+
1895+
checked_out = {"nested/first\noutside", '"quoted"'}
1896+
assert set(repo.index.checkout(["nested", '"quoted"'], force=True)) == checked_out
1897+
assert (nested / "first\noutside").read_bytes() == b"nested"
1898+
assert (tmp_path / '"quoted"').read_bytes() == b"quoted"
1899+
# Neither "nested/first" nor "outside" was requested.
1900+
assert (tmp_path / "outside").read_bytes() == b"local"
1901+
18811902

18821903
class TestIndexUtils:
18831904
@pytest.mark.parametrize("file_path_type", [str, Path])

0 commit comments

Comments
 (0)