From 2135b14863642bbcec02996e7f5e54ac1f77b03a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 19 Aug 2026 14:17:23 +0200 Subject: [PATCH] odb: handle `OBJECT_INFO_DIE_IF_CORRUPT` generically When a lookup with `OBJECT_INFO_DIE_IF_CORRUPT` fails we want to die in case the object exists, but cannot be read. This flag is handled in two different spots right now: - `do_oid_object_info_extended()` calls `has_packed_and_bad()` to check whether the object is known to be corrupt in any packfile. This function reaches into the internals of the packed source and thus breaks the abstraction provided by our object sources. - The loose source handles the flag itself and dies directly in `read_object_info_from_path()`, which means that we die even in cases where another source may still have a good copy of the object. Besides being inconsistent, it also ties us to the specific backend used by the database sources because `has_packed_and_bad()` assumes that they use the "files" backend. Any other backend will instead cause us to die when calling `odb_source_files_downcast()`, even if the object was simply nonexistent. In the preceding commits we've carved out the infrastructure to make this mechanism fully generic. On the one hand, all backends now tell us whether the object is missing or corrupt via their return values. And on the other hand, they have been taught to provide a readable error message to the caller. Adapt `do_oid_object_info_extended()` to use those new mechanisms. This means that we won't die immediately anymore when a loose object is corrupt, and we properly handle backends other than the "files" backend. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 46 +++++++++++++++++++++++++----------- odb/source-loose.c | 10 ++------ packfile.c | 17 ------------- packfile.h | 1 - t/t1060-object-corruption.sh | 18 ++++++++++++++ 5 files changed, 52 insertions(+), 40 deletions(-) diff --git a/odb.c b/odb.c index 83a53f7f6b..6bbea64033 100644 --- a/odb.c +++ b/odb.c @@ -15,7 +15,6 @@ #include "object-name.h" #include "odb.h" #include "odb/source-inmemory.h" -#include "packfile.h" #include "path.h" #include "promisor-remote.h" #include "quote.h" @@ -551,8 +550,11 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database * const struct object_id *oid, struct object_info *oi, unsigned flags) { + struct strbuf corrupt_err = STRBUF_INIT; const struct object_id *real = oid; + enum odb_read_status ret; int already_retried = 0; + bool corrupt = false; if (flags & OBJECT_INFO_LOOKUP_REPLACE) real = lookup_replace_object(odb->repo, oid); @@ -568,9 +570,14 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database * while (1) { struct odb_source *source; - for (source = odb->sources; source; source = source->next) - if (!odb_source_read_object_info(source, real, oi, flags, NULL)) - return 0; + for (source = odb->sources; source; source = source->next) { + ret = odb_source_read_object_info(source, real, oi, flags, + corrupt_err.len ? NULL : &corrupt_err); + if (!ret) + goto out; + if (ret != ODB_READ_NOT_FOUND) + corrupt = true; + } /* * When the object hasn't been found we try a second read and @@ -578,11 +585,15 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database * * caches or reload on-disk state. */ if (!(flags & OBJECT_INFO_QUICK)) { - for (source = odb->sources; source; source = source->next) - if (!odb_source_read_object_info(source, real, oi, - flags | OBJECT_INFO_SECOND_READ, - NULL)) - return 0; + for (source = odb->sources; source; source = source->next) { + ret = odb_source_read_object_info(source, real, oi, + flags | OBJECT_INFO_SECOND_READ, + corrupt_err.len ? NULL : &corrupt_err); + if (!ret) + goto out; + if (ret != ODB_READ_NOT_FOUND) + corrupt = true; + } } /* @@ -605,16 +616,23 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database * } if (flags & OBJECT_INFO_DIE_IF_CORRUPT) { - const struct packed_git *p; if ((flags & OBJECT_INFO_LOOKUP_REPLACE) && !oideq(real, oid)) die(_("replacement %s not found for %s"), oid_to_hex(real), oid_to_hex(oid)); - if ((p = has_packed_and_bad(odb->repo, real))) - die(_("packed object %s (stored in %s) is corrupt"), - oid_to_hex(real), p->pack_name); + if (corrupt) { + if (corrupt_err.len) + die("%s", corrupt_err.buf); + die(_("object %s is corrupt"), oid_to_hex(real)); + } } - return -1; + + ret = corrupt ? ODB_READ_ERROR : ODB_READ_NOT_FOUND; + goto out; } + +out: + strbuf_release(&corrupt_err); + return ret; } static int oid_object_info_convert(struct repository *r, diff --git a/odb/source-loose.c b/odb/source-loose.c index b57ee2701a..540b2dd40d 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -192,15 +192,9 @@ static int read_object_info_from_path(struct odb_source_loose *loose, ret = 0; out: - if (ret && ret != ODB_READ_NOT_FOUND) { - if ((flags & OBJECT_INFO_DIE_IF_CORRUPT)) - die(_("loose object %s (stored in %s) is corrupt"), + if (ret && ret != ODB_READ_NOT_FOUND && errmsg) + strbuf_addf(errmsg, _("loose object %s (stored in %s) is corrupt"), oid_to_hex(oid), path); - if (errmsg) - strbuf_addf(errmsg, _("loose object %s (stored in %s) is corrupt"), - oid_to_hex(oid), path); - } - if (stream_to_end) git_inflate_end(stream_to_end); if (map) diff --git a/packfile.c b/packfile.c index 3cde39a01c..cd38be088d 100644 --- a/packfile.c +++ b/packfile.c @@ -985,23 +985,6 @@ void mark_bad_packed_object(struct packed_git *p, const struct object_id *oid) oidset_insert(&p->bad_objects, oid); } -const struct packed_git *has_packed_and_bad(struct repository *r, - const struct object_id *oid) -{ - struct odb_source *source; - - for (source = r->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); - struct packfile_list_entry *e; - - for (e = files->packed->packs.head; e; e = e->next) - if (oidset_contains(&e->pack->bad_objects, oid)) - return e->pack; - } - - return NULL; -} - off_t get_delta_base(struct packed_git *p, struct pack_window **w_curs, off_t *curpos, diff --git a/packfile.h b/packfile.h index 3229a6ed47..573fe003d0 100644 --- a/packfile.h +++ b/packfile.h @@ -329,7 +329,6 @@ int packed_object_info_with_index_pos(struct odb_source_packed *source, uint32_t *maybe_index_pos, struct object_info *oi); void mark_bad_packed_object(struct packed_git *, const struct object_id *); -const struct packed_git *has_packed_and_bad(struct repository *, const struct object_id *); int has_object_pack(struct repository *r, const struct object_id *oid); int has_object_kept_pack(struct repository *r, const struct object_id *oid, diff --git a/t/t1060-object-corruption.sh b/t/t1060-object-corruption.sh index 502a5ea1c5..d2ef468b45 100755 --- a/t/t1060-object-corruption.sh +++ b/t/t1060-object-corruption.sh @@ -145,4 +145,22 @@ test_expect_success 'partial clone of corrupted repository' ' test_must_fail git -C corrupt-partial checkout --force ' +test_expect_success 'corrupted loose commit can be read from alternate' ' + git init repo-a && + tree=$(git -C repo-a write-tree) && + commit=$(git -C repo-a commit-tree $tree .git/objects/info/alternates && + corrupt_byte "$commit" 1 + ) && + + git -C repo-a cat-file -p "$commit" >expect && + git -C repo-b cat-file -p "$commit" >actual 2>err && + test_cmp expect actual && + test_grep "inflate: data stream error" err +' + test_done