Continue the work in the preceding commit and improve the error on:
$ git hash-object --stdin -w -t garbage --literally </dev/null
$ git fsck
error: hash mismatch for <OID_PATH> (expected <OID>)
error: <OID>: object corrupt or missing: <OID_PATH>
[ other fsck output ]
To instead emit:
$ git fsck
error: <OID>: object is of unknown type 'garbage': <OID_PATH>
[ other fsck output ]
The complaint about a "hash mismatch" was simply an emergent property
of how we'd fall though from read_loose_object() into fsck_loose()
when we didn't get the data we expected. Now we'll correctly note that
the object type is invalid.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
builtin/cat-file.c | 7 +++++--
builtin/fsck.c | 22 ++++++++++++++++++----
object-file.c | 31 +++++++++++++++----------------
object-store.h | 4 ++--
t/t1450-fsck.sh | 23 ++++++++++++++++++++++-
5 files changed, 62 insertions(+), 25 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 5ebf13359e8..1063576a982 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -74,6 +74,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
struct strbuf sb = STRBUF_INIT;
unsigned flags = OBJECT_INFO_LOOKUP_REPLACE;
const char *path = force_path;
+ int ret;
if (unknown_type)
flags |= OBJECT_INFO_ALLOW_UNKNOWN_TYPE;
@@ -92,7 +93,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
switch (opt) {
case 't':
oi.type_name = &sb;
- if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)
+ ret = oid_object_info_extended(the_repository, &oid, &oi, flags);
+ if (!unknown_type && ret < 0)
die("git cat-file: could not get object info");
if (sb.len) {
printf("%s\n", sb.buf);
@@ -103,7 +105,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
case 's':
oi.sizep = &size;
- if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)
+ ret = oid_object_info_extended(the_repository, &oid, &oi, flags);
+ if (!unknown_type && ret < 0)
die("git cat-file: could not get object info");
printf("%"PRIuMAX"\n", (uintmax_t)size);
return 0;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index d92c530863d..c8ab14d1545 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -601,12 +601,26 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *data)
unsigned long size;
void *contents;
int eaten;
-
- if (read_loose_object(path, oid, &type, &size, &contents,
- OBJECT_INFO_ALLOW_UNKNOWN_TYPE) < 0) {
- errors_found |= ERROR_OBJECT;
+ struct strbuf sb = STRBUF_INIT;
+ unsigned int oi_flags = OBJECT_INFO_ALLOW_UNKNOWN_TYPE;
+ struct object_info oi;
+ int found = 0;
+ oi.type_name = &sb;
+ oi.sizep = &size;
+ oi.typep = &type;
+
+ if (read_loose_object(path, oid, &contents, &oi, oi_flags) < 0) {
+ found |= ERROR_OBJECT;
error(_("%s: object corrupt or missing: %s"),
oid_to_hex(oid), path);
+ }
+ if (type < 0) {
+ found |= ERROR_OBJECT;
+ error(_("%s: object is of unknown type '%s': %s"),
+ oid_to_hex(oid), sb.buf, path);
+ }
+ if (found) {
+ errors_found |= ERROR_OBJECT;
return 0; /* keep checking other objects */
}
diff --git a/object-file.c b/object-file.c
index 26560a6281c..e744a06637b 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1323,9 +1323,7 @@ int parse_loose_header(const char *hdr,
* we're obtaining the type using '--allow-unknown-type'
* option.
*/
- if ((flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE) && (type < 0))
- type = 0;
- else if (type < 0)
+ if (type < 0 && !(flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE))
die(_("invalid object type"));
if (oi->typep)
*oi->typep = type;
@@ -1407,14 +1405,17 @@ static int loose_object_info(struct repository *r,
} else if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
status = error(_("unable to unpack %s header"),
oid_to_hex(oid));
- if (status < 0)
+ if (status < 0) {
; /* Do nothing */
- else if (hdrbuf.len) {
+ } else if (hdrbuf.len) {
if ((status = parse_loose_header(hdrbuf.buf, oi, flags)) < 0)
status = error(_("unable to parse %s header with --allow-unknown-type"),
oid_to_hex(oid));
- } else if ((status = parse_loose_header(hdr, oi, flags)) < 0)
- status = error(_("unable to parse %s header"), oid_to_hex(oid));
+ } else {
+ status = parse_loose_header(hdr, oi, flags);
+ if (status < 0 && !(flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE))
+ error(_("unable to parse %s header"), oid_to_hex(oid));
+ }
if (status >= 0 && oi->contentp) {
*oi->contentp = unpack_loose_rest(&stream, hdr,
@@ -2488,9 +2489,8 @@ static int check_stream_oid(git_zstream *stream,
int read_loose_object(const char *path,
const struct object_id *expected_oid,
- enum object_type *type,
- unsigned long *size,
void **contents,
+ struct object_info *oi,
unsigned int oi_flags)
{
int ret = -1;
@@ -2498,8 +2498,8 @@ int read_loose_object(const char *path,
unsigned long mapsize;
git_zstream stream;
char hdr[MAX_HEADER_LEN];
- struct object_info oi = OBJECT_INFO_INIT;
- oi.sizep = size;
+ enum object_type *type = oi->typep;
+ unsigned long *size = oi->sizep;
*contents = NULL;
@@ -2514,9 +2514,9 @@ int read_loose_object(const char *path,
goto out;
}
- *type = parse_loose_header(hdr, &oi, oi_flags);
- if (*type < 0) {
- error(_("unable to parse header of %s"), path);
+ *type = parse_loose_header(hdr, oi, oi_flags);
+ if (*type < 0 && !(oi_flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE)) {
+ error(_("unable to parse header %s"), path);
git_inflate_end(&stream);
goto out;
}
@@ -2532,8 +2532,7 @@ int read_loose_object(const char *path,
goto out;
}
if (check_object_signature(the_repository, expected_oid,
- *contents, *size,
- type_name(*type))) {
+ *contents, *size, oi->type_name->buf)) {
error(_("hash mismatch for %s (expected %s)"), path,
oid_to_hex(expected_oid));
free(*contents);
diff --git a/object-store.h b/object-store.h
index ab86c8bf32c..786c5c34704 100644
--- a/object-store.h
+++ b/object-store.h
@@ -241,11 +241,11 @@ int force_object_loose(const struct object_id *oid, time_t mtime);
*
* Returns 0 on success, negative on error (details may be written to stderr).
*/
+struct object_info;
int read_loose_object(const char *path,
const struct object_id *expected_oid,
- enum object_type *type,
- unsigned long *size,
void **contents,
+ struct object_info *oi,
unsigned int oi_flags);
/* Retry packed storage after checking packed and loose storage */
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 025dd1b491a..214278e134a 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -66,6 +66,25 @@ test_expect_success 'object with hash mismatch' '
)
'
+test_expect_success 'object with hash and type mismatch' '
+ test_create_repo hash-type-mismatch &&
+ (
+ cd hash-type-mismatch &&
+ oid=$(echo blob | git hash-object -w --stdin -t garbage --literally) &&
+ old=$(test_oid_to_path "$oid") &&
+ new=$(dirname $old)/$(test_oid ff_2) &&
+ oid="$(dirname $new)$(basename $new)" &&
+ mv .git/objects/$old .git/objects/$new &&
+ git update-index --add --cacheinfo 100644 $oid foo &&
+ tree=$(git write-tree) &&
+ cmt=$(echo bogus | git commit-tree $tree) &&
+ git update-ref refs/heads/bogus $cmt &&
+ test_must_fail git fsck 2>out &&
+ grep "^error: hash mismatch for " out &&
+ grep "^error: $oid: object is of unknown type '"'"'garbage'"'"'" out
+ )
+'
+
test_expect_success 'branch pointing to non-commit' '
git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
test_when_finished "git update-ref -d refs/heads/invalid" &&
@@ -868,7 +887,9 @@ test_expect_success 'fsck error and recovery on invalid object type' '
empty_blob=$(git -C garbage-type hash-object --stdin -w -t blob </dev/null) &&
garbage_blob=$(git -C garbage-type hash-object --stdin -w -t garbage --literally </dev/null) &&
test_must_fail git -C garbage-type fsck >out 2>err &&
- grep "$garbage_blob: object corrupt or missing:" err &&
+ grep "$garbage_blob: object is of unknown type '"'"'garbage'"'"':" err &&
+ grep error: err >err.errors &&
+ test_line_count = 1 err.errors &&
grep "dangling blob $empty_blob" out
'
--
2.31.1.445.g91d8e479b0a
