Fix invalid oid handling in BlobIO, Reference, RefdbBackend and ODB backends

Invalid oid arguments were silently ignored or left a Python exception set,
which could produce wrong results, SystemError, or crashes. Now:

- Blob__write_to_queue, Reference_init, RefdbBackend_write/delete check
  py_oid_to_git_oid return value and propagate the Python exception.
- Custom ODB backend callbacks return GIT_EUSER on invalid oids.
- Odb_as_iter and OdbBackend_as_iter propagate GIT_EUSER errors without goto.
- OdbBackend_read_prefix/exists_prefix preserve Python exceptions from
  custom backends.

Regression tests added.

Fixes #1478

Assisted-by: Kimi Code

Author: jdavid

CHANGELOG.md

@@ -11,6 +11,12 @@
   [#962](https://github.com/libgit2/pygit2/issues/962)
   [#1100](https://github.com/libgit2/pygit2/pull/1100).
 
+- Fix invalid oid arguments being silently ignored in `BlobIO`, the
+  `Reference` constructor, `RefdbBackend.write()`/`delete()`, and custom
+  `OdbBackend` callbacks; they now raise the appropriate Python exception
+  instead of producing wrong results, `SystemError`, or crashes
+  [#1478](https://github.com/libgit2/pygit2/issues/1478).
+
 
 # 1.20.0 (2026-08-08)
 

src/blob.c

@@ -316,9 +316,11 @@ Blob__write_to_queue(Blob *self, PyObject *args, PyObject *kwds)
     {
         if (py_oid != NULL && py_oid != Py_None)
         {
-            err = py_oid_to_git_oid(py_oid, &opts.attr_commit_id);
-            if (err < 0)
-                return Error_set(err);
+            size_t len = py_oid_to_git_oid(py_oid, &opts.attr_commit_id);
+            if (len == 0) {
+                git_blob_free(blob);
+                return NULL;
+            }
         }
 
         if ((opts.flags & GIT_BLOB_FILTER_NO_SYSTEM_ATTRIBUTES) != 0)

src/odb.c

@@ -113,21 +113,24 @@ Odb_build_as_iter(const git_oid *oid, void *accum)
 PyObject *
 Odb_as_iter(Odb *self)
 {
-    int err;
     PyObject *accum = PyList_New(0);
-    PyObject *ret = NULL;
+    if (accum == NULL)
+        return NULL;
 
-    err = git_odb_foreach(self->odb, Odb_build_as_iter, (void*)accum);
+    int err = git_odb_foreach(self->odb, Odb_build_as_iter, (void*)accum);
+    if (err == GIT_EUSER && PyErr_Occurred()) {
+        Py_DECREF(accum);
+        return NULL;
+    }
     if (err == GIT_EUSER)
-        goto exit;
+        err = GIT_ERROR;
+
     if (err < 0) {
-        ret = Error_set(err);
-        goto exit;
+        Py_DECREF(accum);
+        return Error_set(err);
     }
 
-    ret = PyObject_GetIter(accum);
-
-exit:
+    PyObject *ret = PyObject_GetIter(accum);
     Py_DECREF(accum);
     return ret;
 }

src/odb_backend.c

@@ -124,8 +124,10 @@ pgit_odb_backend_read_prefix(git_oid *oid_out, void **ptr, size_t *sz, git_objec
     }
 
     memcpy(*ptr, bytes, *sz);
-    py_oid_to_git_oid(py_oid_out, oid_out);
+    size_t oid_len = py_oid_to_git_oid(py_oid_out, oid_out);
     Py_DECREF(result);
+    if (oid_len == 0)
+        return GIT_EUSER;
     return 0;
 }
 
@@ -204,8 +206,10 @@ pgit_odb_backend_exists_prefix(git_oid *out, git_odb_backend *_be,
     if (py_oid == NULL)
         return git_error_for_exc();
 
-    py_oid_to_git_oid(py_oid, out);
+    size_t oid_len = py_oid_to_git_oid(py_oid, out);
     Py_DECREF(py_oid);
+    if (oid_len == 0)
+        return GIT_EUSER;
     return 0;
 }
 
@@ -224,15 +228,33 @@ pgit_odb_backend_foreach(git_odb_backend *_be,
     PyObject *item;
     git_oid oid;
     pgit_odb_backend *be = (pgit_odb_backend *)_be;
-    PyObject *iterator = PyObject_GetIter((PyObject *)be->py_backend);
-    assert(iterator);
+
+    /* Call the Python __iter__ method directly. PyObject_GetIter would invoke
+     * the C tp_iter slot (OdbBackend_as_iter), which calls this function back
+     * and causes infinite recursion for Python backends. */
+    PyObject *iter_method = PyObject_GetAttrString((PyObject *)be->py_backend, "__iter__");
+    if (iter_method == NULL)
+        return git_error_for_exc();
+
+    PyObject *iterator = PyObject_CallObject(iter_method, NULL);
+    Py_DECREF(iter_method);
+    if (iterator == NULL)
+        return git_error_for_exc();
 
     while ((item = PyIter_Next(iterator))) {
-        py_oid_to_git_oid(item, &oid);
-        cb(&oid, payload);
+        size_t len = py_oid_to_git_oid(item, &oid);
         Py_DECREF(item);
+        if (len == 0) {
+            Py_DECREF(iterator);
+            return GIT_EUSER;
+        }
+        if (cb(&oid, payload) != 0) {
+            Py_DECREF(iterator);
+            return GIT_EUSER;
+        }
     }
 
+    Py_DECREF(iterator);
     return git_error_for_exc();
 }
 
@@ -278,7 +300,7 @@ OdbBackend_init(OdbBackend *self, PyObject *args, PyObject *kwds)
 //  custom_backend->backend.freshen = pgit_odb_backend_freshen;
 //  custom_backend->backend.writestream = pgit_odb_backend_writestream;
 //  custom_backend->backend.readstream = pgit_odb_backend_readstream;
-    if (PyIter_Check((PyObject *)self))
+    if (PyObject_HasAttrString((PyObject *)self, "__iter__"))
         custom_backend->backend.foreach = pgit_odb_backend_foreach;
 
     // Cross reference (don't incref because it's something internal)
@@ -321,20 +343,23 @@ PyObject *
 OdbBackend_as_iter(OdbBackend *self)
 {
     PyObject *accum = PyList_New(0);
-    PyObject *iter = NULL;
+    if (accum == NULL)
+        return NULL;
 
     int err = self->odb_backend->foreach(self->odb_backend, OdbBackend_build_as_iter, (void*)accum);
+    if (err == GIT_EUSER && PyErr_Occurred()) {
+        Py_DECREF(accum);
+        return NULL;
+    }
     if (err == GIT_EUSER)
-        goto exit;
+        err = GIT_ERROR;
 
     if (err < 0) {
-        Error_set(err);
-        goto exit;
+        Py_DECREF(accum);
+        return Error_set(err);
     }
 
-    iter = PyObject_GetIter(accum);
-
-exit:
+    PyObject *iter = PyObject_GetIter(accum);
     Py_DECREF(accum);
     return iter;
 }
@@ -397,8 +422,9 @@ OdbBackend_read_prefix(OdbBackend *self, PyObject *py_hex)
 
     err = self->odb_backend->read_prefix(&oid_out, &data, &size, &type, self->odb_backend, &oid, len);
     if (err != 0) {
-        Error_set_oid(err, &oid, len);
-        return NULL;
+        if (err == GIT_EUSER && PyErr_Occurred())
+            return NULL;
+        return Error_set_oid(err, &oid, len);
     }
 
     PyObject *py_oid_out = git_oid_to_python(&oid_out);
@@ -492,8 +518,11 @@ OdbBackend_exists_prefix(OdbBackend *self, PyObject *py_hex)
     git_oid out;
     result = self->odb_backend->exists_prefix(&out, self->odb_backend, &oid, len);
 
-    if (result < 0)
+    if (result < 0) {
+        if (result == GIT_EUSER && PyErr_Occurred())
+            return NULL;
         return Error_set(result);
+    }
 
     return git_oid_to_python(&out);
 }

src/refdb_backend.c

@@ -711,7 +711,8 @@ RefdbBackend_write(RefdbBackend *self, PyObject *args)
         return NULL;
 
     if ((PyObject *)py_old != Py_None) {
-        py_oid_to_git_oid(py_old, &_old);
+        if (py_oid_to_git_oid(py_old, &_old) == 0)
+            return NULL;
         old = &_old;
     }
 
@@ -785,7 +786,8 @@ RefdbBackend_delete(RefdbBackend *self, PyObject *args)
         return NULL;
 
     if (py_old_id != Py_None) {
-        py_oid_to_git_oid(py_old_id, &old_id);
+        if (py_oid_to_git_oid(py_old_id, &old_id) == 0)
+            return NULL;
         err = self->refdb_backend->del(self->refdb_backend,
                 ref_name, &old_id, old_target);
     } else {

src/reference.c

@@ -165,9 +165,11 @@ Reference_init(Reference *self, PyObject *args, PyObject *kwds)
         return -1;
     }
 
-    py_oid_to_git_oid(py_oid, &oid);
+    if (py_oid_to_git_oid(py_oid, &oid) == 0)
+        return -1;
     if (py_peel != Py_None) {
-        py_oid_to_git_oid(py_peel, &peel);
+        if (py_oid_to_git_oid(py_peel, &peel) == 0)
+            return -1;
     }
 
     self->reference = git_reference__alloc(name, &oid,

test/test_blob.py

@@ -34,7 +34,7 @@
 
 import pygit2
 from pygit2 import Repository
-from pygit2.enums import ObjectType
+from pygit2.enums import BlobFilter, ObjectType
 
 from . import utils
 
@@ -266,3 +266,43 @@ def test_blobio_filtered(testrepo: Repository) -> None:
     with pygit2.BlobIO(blob, as_path='bye.txt') as reader:
         assert b'bye world\n' == reader.read()
     assert not reader.raw._thread.is_alive()  # type: ignore[attr-defined]
+
+
+def test_blob_write_to_queue_invalid_commit_id_type(testrepo: Repository) -> None:
+    # Regression test (issue #1478): an invalid commit_id type must raise
+    # TypeError instead of being ignored and leaving an exception set.
+    queue: Queue[bytes] = Queue()
+    ready = Event()
+    done = Event()
+    blob_oid = testrepo.create_blob_fromworkdir('bye.txt')
+    blob = testrepo[blob_oid]
+    assert isinstance(blob, pygit2.Blob)
+    with pytest.raises(TypeError):
+        blob._write_to_queue(
+            queue,
+            ready,
+            done,
+            as_path='bye.txt',
+            flags=BlobFilter.ATTRIBUTES_FROM_COMMIT,
+            commit_id=1234,  # type: ignore
+        )
+
+
+def test_blob_write_to_queue_invalid_commit_id_str(testrepo: Repository) -> None:
+    # Regression test (issue #1478): a malformed commit_id string must raise
+    # InvalidError instead of being ignored and leaving an exception set.
+    queue: Queue[bytes] = Queue()
+    ready = Event()
+    done = Event()
+    blob_oid = testrepo.create_blob_fromworkdir('bye.txt')
+    blob = testrepo[blob_oid]
+    assert isinstance(blob, pygit2.Blob)
+    with pytest.raises(pygit2.InvalidError):
+        blob._write_to_queue(
+            queue,
+            ready,
+            done,
+            as_path='bye.txt',
+            flags=BlobFilter.ATTRIBUTES_FROM_COMMIT,
+            commit_id='not-a-valid-oid',  # type: ignore[arg-type]
+        )

test/test_odb_backend.py

@@ -169,3 +169,50 @@ def test_repo_read(repo: Repository) -> None:
     ab = repo[BLOB_OID]
     a = repo[BLOB_HEX]
     assert ab == a
+
+
+class BadOidReadPrefixBackend(ProxyBackend):
+    def read_prefix_cb(self, oid: Oid | str) -> tuple[int, bytes, Oid | str]:  # type: ignore[override]
+        return (ObjectType.BLOB, b'bad', 'not-a-valid-oid')
+
+
+class BadOidExistsPrefixBackend(ProxyBackend):
+    def exists_prefix_cb(self, oid: Oid | str) -> Oid | str:  # type: ignore[override]
+        return 'not-a-valid-oid'
+
+
+class BadOidIterBackend(ProxyBackend):
+    def __iter__(self) -> Iterator[Oid | str]:  # type: ignore[override]
+        yield 'not-a-valid-oid'
+
+
+def test_read_prefix_cb_bad_oid(barerepo: Repository) -> None:
+    # Regression test (issue #1478): an ODB backend returning an invalid oid
+    # from read_prefix_cb must raise InvalidError instead of silently returning
+    # garbage data.
+    path = Path(barerepo.path) / 'objects'
+    backend = BadOidReadPrefixBackend(pygit2.OdbBackendPack(path))
+    with pytest.raises(pygit2.InvalidError):
+        backend.read_prefix(BLOB_HEX[:4])
+
+
+def test_exists_prefix_cb_bad_oid(barerepo: Repository) -> None:
+    # Regression test (issue #1478): an ODB backend returning an invalid oid
+    # from exists_prefix_cb must raise InvalidError instead of silently returning
+    # garbage data.
+    path = Path(barerepo.path) / 'objects'
+    backend = BadOidExistsPrefixBackend(pygit2.OdbBackendPack(path))
+    with pytest.raises(pygit2.InvalidError):
+        backend.exists_prefix(BLOB_HEX[:4])
+
+
+def test_foreach_cb_bad_oid(barerepo: Repository) -> None:
+    # Regression test (issue #1478): an ODB backend yielding an invalid oid
+    # during iteration must raise InvalidError instead of crashing or returning
+    # garbage data.
+    path = Path(barerepo.path) / 'objects'
+    backend = BadOidIterBackend(pygit2.OdbBackendPack(path))
+    odb = pygit2.Odb()
+    odb.add_backend(backend, 1)
+    with pytest.raises(pygit2.InvalidError):
+        next(iter(odb))

test/test_refdb_backend.py

@@ -224,6 +224,42 @@ def test_write(repo: Repository) -> None:
     assert repo.backend.lookup('refs/heads/test-write').target == master.target
 
 
+def test_write_invalid_old_type(repo: Repository) -> None:
+    # Regression test (issue #1478): RefdbBackend.write must raise TypeError
+    # when old is not a valid oid, not silently ignore the bad argument.
+    master = repo.backend.lookup('refs/heads/master')
+    commit = repo[master.target]
+    ref = pygit2.Reference('refs/heads/test-write', master.target, None)
+    with pytest.raises(TypeError):
+        repo.backend.write(ref, False, commit.author, 'Create test-write', 1234, None)  # type: ignore
+
+
+def test_write_invalid_old_str(repo: Repository) -> None:
+    # Regression test (issue #1478): RefdbBackend.write must raise InvalidError
+    # when old is a malformed oid string, not silently ignore the bad argument.
+    master = repo.backend.lookup('refs/heads/master')
+    commit = repo[master.target]
+    ref = pygit2.Reference('refs/heads/test-write', master.target, None)
+    with pytest.raises(pygit2.InvalidError):
+        repo.backend.write(
+            ref, False, commit.author, 'Create test-write', 'not-a-valid-oid', None
+        )
+
+
+def test_delete_invalid_old_type(repo: Repository) -> None:
+    # Regression test (issue #1478): RefdbBackend.delete must raise TypeError
+    # when old_id is not a valid oid, not silently ignore the bad argument.
+    with pytest.raises(TypeError):
+        repo.backend.delete('refs/heads/master', 1234, None)  # type: ignore
+
+
+def test_delete_invalid_old_str(repo: Repository) -> None:
+    # Regression test (issue #1478): RefdbBackend.delete must raise InvalidError
+    # when old_id is a malformed oid string, not silently ignore the bad argument.
+    with pytest.raises(pygit2.InvalidError):
+        repo.backend.delete('refs/heads/master', 'not-a-valid-oid', None)
+
+
 def test_rename(repo: Repository) -> None:
     old_ref = repo.backend.lookup('refs/heads/i18n')
     target = repo.get(old_ref.target)