Skip to content

Commit b195d86

Browse files
codexByron
authored andcommitted
fix(diff): preserve bytes in text paths and rename metadata
Diff raw paths preserved bytes, but their text properties decoded with replacement characters. Renaming `legacy-\xff` to `legacy-\xfe` collapsed both names to the same string, hid the rename and gave diff blobs unusable paths. Patch rename fields also retained Git's quoted spelling. Use the existing lossless decoder for text paths and submodule matching, compare raw rename names, and unquote patch rename fields through the existing path decoder. Document the intentional change from replacement characters to surrogate escapes in public text paths. The new raw/patch rename regressions fail before this change and pass with both `core.quotePath` settings on CLI and Gix. Git 2.54.0 supplies the fixture renames; native operation counts confirm the raw Gix path. All 49 Diff tests pass on each backend. Ruff, ShellCheck and whitespace checks pass.
1 parent 64e169c commit b195d86

4 files changed

Lines changed: 42 additions & 10 deletions

File tree

‎doc/source/changes.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ API changes
5757
* Raw diffs preserve non-UTF-8 filename bytes in ``a_rawpath``, ``b_rawpath``,
5858
and raw rename fields. Staged-file diffs no longer fail while decoding legacy
5959
filenames; blob contents remain byte streams regardless of their encoding.
60+
Text paths and rename properties use ``surrogateescape`` instead of replacing
61+
undecodable bytes. Diff blob paths can be used for tree lookup, and renames
62+
between distinct byte names are detected correctly. Patch rename fields
63+
contain unquoted filename bytes.
6064
* Tree serialization now delegates construction to Git or the supported Gix
6165
backend. ``Tree.cache`` and ``TreeModifier`` edit one tree's in-memory entries;
6266
object IDs and parent trees do not change automatically. Prefer

‎git/diff.py‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
from git import _backend
1212
from git.cmd import Git, handle_process_output
13-
from git.compat import defenc
13+
from git.compat import defenc, safe_decode
1414
from git.objects.base import IndexObject
1515
from git.objects.blob import Blob
1616
from git.objects.util import mode_str_to_int
@@ -486,7 +486,7 @@ def __init__(
486486
# we need to overwrite "repo" to the corresponding submodule's repo instead.
487487
if repo and a_rawpath and (self.a_mode == 0o160000 or self.b_mode == 0o160000):
488488
for submodule in repo.submodules:
489-
if submodule.path == a_rawpath.decode(defenc, "replace"):
489+
if submodule.path == safe_decode(a_rawpath):
490490
if submodule.module_exists():
491491
repo = submodule.module()
492492
break
@@ -582,24 +582,24 @@ def __str__(self) -> str:
582582

583583
@property
584584
def a_path(self) -> Optional[str]:
585-
return self.a_rawpath.decode(defenc, "replace") if self.a_rawpath else None
585+
return safe_decode(self.a_rawpath) if self.a_rawpath else None
586586

587587
@property
588588
def b_path(self) -> Optional[str]:
589-
return self.b_rawpath.decode(defenc, "replace") if self.b_rawpath else None
589+
return safe_decode(self.b_rawpath) if self.b_rawpath else None
590590

591591
@property
592592
def rename_from(self) -> Optional[str]:
593-
return self.raw_rename_from.decode(defenc, "replace") if self.raw_rename_from else None
593+
return safe_decode(self.raw_rename_from) if self.raw_rename_from else None
594594

595595
@property
596596
def rename_to(self) -> Optional[str]:
597-
return self.raw_rename_to.decode(defenc, "replace") if self.raw_rename_to else None
597+
return safe_decode(self.raw_rename_to) if self.raw_rename_to else None
598598

599599
@property
600600
def renamed_file(self) -> bool:
601601
""":return: ``True`` if the blob of our diff has been renamed"""
602-
return self.rename_from != self.rename_to
602+
return self.raw_rename_from != self.raw_rename_to
603603

604604
@classmethod
605605
def _pick_best_path(cls, path_match: bytes, rename_match: bytes, path_fallback_match: bytes) -> Optional[bytes]:
@@ -742,8 +742,8 @@ def finalize_process_with_stderr(proc: Union["Popen", "Git.AutoInterrupt"]) -> N
742742
new_file,
743743
deleted_file,
744744
copied_file,
745-
rename_from,
746-
rename_to,
745+
decode_path(rename_from, has_ab_prefix=False) if rename_from else None,
746+
decode_path(rename_to, has_ab_prefix=False) if rename_to else None,
747747
None,
748748
None,
749749
None,

‎test/test_diff.py‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import pytest
1010

1111
from git import NULL_TREE, Diff, DiffIndex, Diffable, Repo
12+
from git.compat import safe_encode
1213
from git.diff import decode_path
1314
from git.exc import UnsafeOptionError
1415

@@ -36,6 +37,22 @@ def test_raw_diff_preserves_non_utf8_paths(change):
3637
assert (diffs[0].raw_rename_from, diffs[0].raw_rename_to) == (before, after)
3738

3839

40+
@pytest.mark.parametrize("create_patch", [False, True])
41+
@pytest.mark.parametrize("quote_path", ["false", "true"])
42+
def test_diff_paths_and_renames_preserve_non_utf8_bytes(create_patch, quote_path):
43+
with fixture_repo("diff-byte-paths.sh", quote_path) as repo:
44+
before, after = repo.commit("HEAD^"), repo.head.commit
45+
(diff,) = before.diff(after, create_patch=create_patch)
46+
assert (diff.a_rawpath, diff.b_rawpath) == (b"legacy-\xff", b"legacy-\xfe")
47+
assert (diff.raw_rename_from, diff.raw_rename_to) == (diff.a_rawpath, diff.b_rawpath)
48+
assert (safe_encode(diff.a_path), safe_encode(diff.b_path)) == (diff.a_rawpath, diff.b_rawpath)
49+
assert (safe_encode(diff.rename_from), safe_encode(diff.rename_to)) == (diff.a_rawpath, diff.b_rawpath)
50+
assert diff.renamed_file
51+
if not create_patch: # Exact-rename patches omit the blob IDs.
52+
assert before.tree[diff.a_blob.path].binsha == diff.a_blob.binsha
53+
assert after.tree[diff.b_blob.path].binsha == diff.b_blob.binsha
54+
55+
3956
@ddt.ddt
4057
class TestDiff(TestBase):
4158
def setUp(self):
@@ -351,7 +368,7 @@ def test_diff_unsafe_paths(self):
351368
self.assertEqual(res[8].b_path, "path/¯\\_(ツ)_|¯")
352369
self.assertEqual(res[9].b_path, "path/💩.txt")
353370
self.assertEqual(res[9].b_rawpath, b"path/\xf0\x9f\x92\xa9.txt")
354-
self.assertEqual(res[10].b_path, "path/�-invalid-unicode-path.txt")
371+
self.assertEqual(safe_encode(res[10].b_path), b"path/\x80-invalid-unicode-path.txt")
355372
self.assertEqual(res[10].b_rawpath, b"path/\x80-invalid-unicode-path.txt")
356373

357374
# The "Moves"

‎tests/fixtures/diff-byte-paths.sh‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#!/usr/bin/env bash
2+
set -eu
3+
git init --initial-branch=main repo
4+
cd repo
5+
git config core.quotePath "$1"
6+
blob=$(printf 'content\n' | git hash-object -w --stdin)
7+
before=$(printf '100644 blob %s\tlegacy-\377\0' "$blob" | git mktree -z)
8+
after=$(printf '100644 blob %s\tlegacy-\376\0' "$blob" | git mktree -z)
9+
parent=$(printf 'before\n' | git commit-tree "$before")
10+
commit=$(printf 'after\n' | git commit-tree "$after" -p "$parent")
11+
git update-ref refs/heads/main "$commit"

0 commit comments

Comments
 (0)