FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: validate object types when loading binary SHAs (#2254) · gitpython-developers/GitPython@ca55dbf · GitHub

  1. gitpython-developers
  2. GitPython

Repository navigation

Commit ca55dbf

Browse files
codex
authored and
Byron
committed
fix: validate object types when loading binary SHAs (#2254)
Constructing a `Commit` with an annotated tag's `binsha` let the commit parser interpret tag headers. Accessing its tree then tried to parse a commit as tree data and raised an unrelated `IndexError`. Validate the object database's type before parsing object data or exposing its size and streams. Share the check across `Commit`, `Tree`, `TagObject`, `Blob`, and the raw tree traversals used by index merging. Report the SHA, actual type, and expected type in a `ValueError`. Constructors remain lazy so placeholders and objects outside the current database still work. Drain rejected streams in bounded chunks so a retained exception does not desynchronize subsequent `git cat-file` requests. Also preserve binary SHAs in `IndexFile.new()` instead of converting them to their Python string representation. Accept binary and hexadecimal bytes alongside hex strings and `Tree` objects. Represent submodule diff entries with `IndexObject`, retaining their path, mode, and access to the submodule's commit data without labeling those commits as blobs. Extend existing object, index, and submodule diff tests rather than adding dedicated tests. Update the existing commit stream mock to return an `OStream`, and remove the obsolete `IndexFile.new()` assignment diagnostic from the `basedpyright` baseline. Git reference: commit `d38352cd43ab9745686d697872408bc3249a153f` in the local Git reference checkout. Its `repo_parse_commit_internal()` in `commit.c`, `repo_parse_tree_gently()` in `tree.c`, and `parse_tag()` in `tag.c` check object types before parsing their contents. Validation: - The extended `test_base_object` and `test_index_new` failed before the fix. - The affected object, blob, commit, tree, index, helper, reference, diff, and no-fetch submodule suites passed 227 tests, with 2 skipped, across the suite run and the staged-file test's rerun with `init.defaultBranch=master`. Test commands disabled `commit.gpgSign`; tests needing local sockets or the linked object store ran outside the sandbox. - The issue reproduction, binary-SHA index creation, and generic/lazy objects were checked with both `GitCmdObjectDB` and `GitDB`. - `ruff check .`, `ruff format --check .`, `mypy` (46 source files), and `basedpyright --warnings` passed.
1 parent 71b9545 commit ca55dbf

13 files changed

Lines changed: 84 additions & 31 deletions

File tree

‎.basedpyright/baseline.json‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,6 @@
7777
"lineCount": 1
7878
}
7979
},
80-
{
81-
"code": "reportAssignmentType",
82-
"range": {
83-
"startColumn": 38,
84-
"endColumn": 76,
85-
"lineCount": 1
86-
}
87-
},
8880
{
8981
"code": "reportArgumentType",
9082
"range": {

‎git/diff.py‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
from git.cmd import Git, handle_process_output
1313
from git.compat import defenc
14+
from git.objects.base import IndexObject
1415
from git.objects.blob import Blob
1516
from git.objects.util import mode_str_to_int
1617
from git.util import finalize_process, hex_to_bin
@@ -35,7 +36,6 @@
3536
if TYPE_CHECKING:
3637
from subprocess import Popen
3738

38-
from git.objects.base import IndexObject
3939
from git.objects.commit import Commit
4040
from git.objects.tree import Tree
4141
from git.repo.base import Repo
@@ -378,6 +378,10 @@ class Diff:
378378
Diffs keep information about the changed blob objects, the file mode, renames,
379379
deletions and new files.
380380
381+
For submodule changes, ``a_blob`` and ``b_blob`` are
382+
:class:`~git.objects.base.IndexObject` instances whose SHAs refer to commits in
383+
the submodule repository.
384+
381385
There are a few cases where ``None`` has to be expected as member variable value:
382386
383387
New File::
@@ -481,17 +485,22 @@ def __init__(
481485
repo = submodule.module()
482486
break
483487

488+
# Gitlinks reference commits; generic index objects preserve their path and mode.
484489
self.a_blob: Union["IndexObject", None]
485490
if a_blob_id is None or a_blob_id == self.NULL_HEX_SHA:
486491
self.a_blob = None
487492
else:
488-
self.a_blob = Blob(repo, hex_to_bin(a_blob_id), mode=self.a_mode, path=self.a_path)
493+
self.a_blob = (IndexObject if self.a_mode == 0o160000 else Blob)(
494+
repo, hex_to_bin(a_blob_id), mode=self.a_mode, path=self.a_path
495+
)
489496

490497
self.b_blob: Union["IndexObject", None]
491498
if b_blob_id is None or b_blob_id == self.NULL_HEX_SHA:
492499
self.b_blob = None
493500
else:
494-
self.b_blob = Blob(repo, hex_to_bin(b_blob_id), mode=self.b_mode, path=self.b_path)
501+
self.b_blob = (IndexObject if self.b_mode == 0o160000 else Blob)(
502+
repo, hex_to_bin(b_blob_id), mode=self.b_mode, path=self.b_path
503+
)
495504

496505
self.new_file: bool = new_file
497506
self.deleted_file: bool = deleted_file

‎git/index/base.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ def merge_tree(
310310
return self
311311

312312
@classmethod
313-
def new(cls, repo: "Repo", *tree_sha: Union[str, Tree]) -> "IndexFile":
313+
def new(cls, repo: "Repo", *tree_sha: Union[str, bytes, Tree]) -> "IndexFile":
314314
"""Merge the given treeish revisions into a new index which is returned.
315315
316316
This method behaves like ``git-read-tree --aggressive`` when doing the merge.
@@ -326,7 +326,9 @@ def new(cls, repo: "Repo", *tree_sha: Union[str, Tree]) -> "IndexFile":
326326
If you intend to write such a merged Index, supply an alternate
327327
``file_path`` to its :meth:`write` method.
328328
"""
329-
tree_sha_bytes: List[bytes] = [to_bin_sha(str(t)) for t in tree_sha]
329+
tree_sha_bytes: List[bytes] = [
330+
to_bin_sha(t if isinstance(t, bytes) else str(t).encode("ascii")) for t in tree_sha
331+
]
330332
base_entries = aggressive_tree_merge(repo.odb, tree_sha_bytes)
331333

332334
inst = cls(repo)

‎git/objects/base.py‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from git.exc import WorkTreeRepositoryUnsupported
1313
from git.util import LazyMixin, bin_to_hex, join_path_native, stream_copy
1414

15-
from .util import get_object_type_by_name
15+
from .util import _assert_object_type, get_object_type_by_name
1616

1717
# typing ------------------------------------------------------------------
1818

@@ -107,6 +107,10 @@ def __init__(self, repo: "Repo", binsha: bytes) -> None:
107107
108108
:param binsha:
109109
20 byte SHA1
110+
111+
:note:
112+
Object data is loaded lazily. If `binsha` refers to a different object
113+
type, accessing that data raises :exc:`ValueError`.
110114
"""
111115
super().__init__()
112116
self.repo = repo
@@ -155,6 +159,7 @@ def _set_cache_(self, attr: str) -> None:
155159
"""Retrieve object information."""
156160
if attr == "size":
157161
oinfo = self.repo.odb.info(self.binsha)
162+
_assert_object_type(oinfo, self.type)
158163
self.size = oinfo.size # type: int
159164
else:
160165
super()._set_cache_(attr)
@@ -198,7 +203,9 @@ def data_stream(self) -> "OStream":
198203
:note:
199204
Returned streams must be read in order.
200205
"""
201-
return self.repo.odb.stream(self.binsha)
206+
ostream = self.repo.odb.stream(self.binsha)
207+
_assert_object_type(ostream, self.type)
208+
return ostream
202209

203210
def stream_data(self, ostream: SupportsWrite[bytes]) -> "Object":
204211
"""Write our data directly to the given output stream.
@@ -209,7 +216,7 @@ def stream_data(self, ostream: SupportsWrite[bytes]) -> "Object":
209216
:return:
210217
self
211218
"""
212-
istream = self.repo.odb.stream(self.binsha)
219+
istream = self.data_stream
213220
stream_copy(istream, ostream)
214221
return self
215222

‎git/objects/commit.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ def replace(self, **kwargs: Any) -> "Commit":
242242
def _set_cache_(self, attr: str) -> None:
243243
if attr in Commit.__slots__:
244244
# Read the data in a chunk, its faster - then provide a file wrapper.
245-
_binsha, _typename, self.size, stream = self.repo.odb.stream(self.binsha)
245+
_binsha, _typename, self.size, stream = self.data_stream
246246
self._deserialize(BytesIO(stream.read()))
247247
else:
248248
super()._set_cache_(attr)

‎git/objects/fun.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
from git.compat import safe_decode, defenc
1616

17+
from .util import _assert_object_type
18+
1719
# typing ----------------------------------------------
1820

1921
from typing import (
@@ -200,8 +202,10 @@ def traverse_trees_recursive(
200202
if tree_sha is None:
201203
data: List[EntryTupOrNone] = []
202204
else:
205+
stream = odb.stream(tree_sha)
206+
_assert_object_type(stream, "tree")
203207
# Make new list for typing as list invariant.
204-
data = list(tree_entries_from_data(odb.stream(tree_sha).read()))
208+
data = list(tree_entries_from_data(stream.read()))
205209
# END handle muted trees
206210
trees_data.append(data)
207211
# END for each sha to get data for
@@ -268,7 +272,9 @@ def traverse_tree_recursive(odb: "ObjectDBR", tree_sha: bytes, path_prefix: str)
268272
Prefix to prepend to the front of all returned paths.
269273
"""
270274
entries = []
271-
data = tree_entries_from_data(odb.stream(tree_sha).read())
275+
stream = odb.stream(tree_sha)
276+
_assert_object_type(stream, "tree")
277+
data = tree_entries_from_data(stream.read())
272278

273279
# Unpacking/packing is faster than accessing individual items.
274280
for sha, mode, name in data:

‎git/objects/tag.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ def __init__(
105105
def _set_cache_(self, attr: str) -> None:
106106
"""Cache all our attributes at once."""
107107
if attr in TagObject.__slots__:
108-
ostream = self.repo.odb.stream(self.binsha)
108+
ostream = self.data_stream
109109
lines: List[str] = ostream.read().decode(defenc, "replace").splitlines()
110110

111111
_obj, hexsha = lines[0].split(" ")

‎git/objects/tree.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ def _get_intermediate_items(
208208
def _set_cache_(self, attr: str) -> None:
209209
if attr == "_cache":
210210
# Set the data when we need it.
211-
ostream = self.repo.odb.stream(self.binsha)
211+
ostream = self.data_stream
212212
self._cache: List[TreeCacheTup] = tree_entries_from_data(ostream.read())
213213
else:
214214
super()._set_cache_(attr)

‎git/objects/util.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import time
2929
import warnings
3030

31+
from gitdb.base import OStream
32+
3133
from git.util import Actor, IterableList, IterableObj
3234

3335
# typing ------------------------------------------------------------
@@ -54,6 +56,8 @@
5456
from io import BytesIO, StringIO
5557
from subprocess import Popen
5658

59+
from gitdb.base import OInfo
60+
5761
from git.types import Protocol, runtime_checkable
5862

5963
from .blob import Blob
@@ -88,6 +92,19 @@ class TraverseNT(NamedTuple):
8892
# { Functions
8993

9094

95+
def _assert_object_type(oinfo: "OInfo", expected_type: Union[str, None]) -> None:
96+
"""Reject mismatched object types before interpreting their data."""
97+
if expected_type is None or oinfo.type == expected_type.encode("ascii"):
98+
return
99+
if isinstance(oinfo, OStream):
100+
# Drain the cat-file response even if the exception keeps the stream alive.
101+
while oinfo.read(8192):
102+
pass
103+
raise ValueError(
104+
"Object %s is a %s, not a %s" % (oinfo.hexsha.decode("ascii"), oinfo.type.decode("ascii"), expected_type)
105+
)
106+
107+
91108
def mode_str_to_int(modestr: Union[bytes, str]) -> int:
92109
"""Convert mode bits from an octal mode string to an integer mode for git.
93110

‎test/test_base.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
55

66
import gc
7+
from io import BytesIO
78
import os
89
import os.path as osp
910
import sys
@@ -76,6 +77,21 @@ def test_base_object(self):
7677
# Remove the file this way, instead of with a context manager or "finally",
7778
# so it is only removed on success, and we can inspect the file on failure.
7879
os.remove(tmpfile.name)
80+
81+
for wrong_type, attr in ((Blob, "data_stream"), (Tree, "_cache"), (Commit, "tree"), (TagObject, "object")):
82+
if wrong_type is obj_type:
83+
continue
84+
invalid = wrong_type(self.rorepo, binsha)
85+
for invalid_attr in ("size", attr):
86+
with self.assertRaisesRegex(ValueError, f"{hexsha}.*{typename}.*{wrong_type.type}"):
87+
getattr(invalid, invalid_attr)
88+
try:
89+
invalid.stream_data(BytesIO())
90+
except ValueError:
91+
# The next read must work even while the exception retains the stream.
92+
self.assertEqual(item.data_stream.read(), data)
93+
else:
94+
self.fail("Expected ValueError for mismatched object type")
7995
# END for each object type to create
8096

8197
# Each has a unique sha.

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL