Skip to content

Commit 061a21d

Browse files
gitstergitster
committed
Merge branch 'ab/fsck-unexpected-type'
"git fsck" has been taught to report mismatch between expected and actual types of an object better. * ab/fsck-unexpected-type: fsck: report invalid object type-path combinations fsck: don't hard die on invalid object types object-file.c: stop dying in parse_loose_header() object-file.c: return ULHR_TOO_LONG on "header too long" object-file.c: use "enum" return type for unpack_loose_header() object-file.c: simplify unpack_loose_short_header() object-file.c: make parse_loose_header_extended() public object-file.c: return -1, not "status" from unpack_loose_header() object-file.c: don't set "typep" when returning non-zero cat-file tests: test for current --allow-unknown-type behavior cat-file tests: add corrupt loose object test cat-file tests: test for missing/bogus object with -t, -s and -p cat-file tests: move bogus_* variable declarations earlier fsck tests: test for garbage appended to a loose object fsck tests: test current hash/type mismatch behavior fsck tests: refactor one test to use a sub-repo fsck tests: add test for fsck-ing an unknown type
2 parents 9d530dc + 96e41f5 commit 061a21d

File tree

13 files changed

+478
-162
lines changed

13 files changed

+478
-162
lines changed

‎builtin/fast-export.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ static void export_blob(const struct object_id *oid)
312312
if (!buf)
313313
die("could not read blob %s", oid_to_hex(oid));
314314
if (check_object_signature(the_repository, oid, buf, size,
315-
type_name(type)) < 0)
315+
type_name(type), NULL) < 0)
316316
die("oid mismatch in blob %s", oid_to_hex(oid));
317317
object = parse_object_buffer(the_repository, oid, type,
318318
size, buf, &eaten);

‎builtin/fsck.c‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -593,18 +593,43 @@ static void get_default_heads(void)
593593
}
594594
}
595595

596+
struct for_each_loose_cb
597+
{
598+
struct progress *progress;
599+
struct strbuf obj_type;
600+
};
601+
596602
static int fsck_loose(const struct object_id *oid, const char *path, void *data)
597603
{
604+
struct for_each_loose_cb *cb_data = data;
598605
struct object *obj;
599-
enum object_type type;
606+
enum object_type type = OBJ_NONE;
600607
unsigned long size;
601608
void *contents;
602609
int eaten;
610+
struct object_info oi = OBJECT_INFO_INIT;
611+
struct object_id real_oid = *null_oid();
612+
int err = 0;
603613

604-
if (read_loose_object(path, oid, &type, &size, &contents) < 0) {
614+
strbuf_reset(&cb_data->obj_type);
615+
oi.type_name = &cb_data->obj_type;
616+
oi.sizep = &size;
617+
oi.typep = &type;
618+
619+
if (read_loose_object(path, oid, &real_oid, &contents, &oi) < 0) {
620+
if (contents && !oideq(&real_oid, oid))
621+
err = error(_("%s: hash-path mismatch, found at: %s"),
622+
oid_to_hex(&real_oid), path);
623+
else
624+
err = error(_("%s: object corrupt or missing: %s"),
625+
oid_to_hex(oid), path);
626+
}
627+
if (type != OBJ_NONE && type < 0)
628+
err = error(_("%s: object is of unknown type '%s': %s"),
629+
oid_to_hex(&real_oid), cb_data->obj_type.buf,
630+
path);
631+
if (err < 0) {
605632
errors_found |= ERROR_OBJECT;
606-
error(_("%s: object corrupt or missing: %s"),
607-
oid_to_hex(oid), path);
608633
return 0; /* keep checking other objects */
609634
}
610635

@@ -640,15 +665,21 @@ static int fsck_cruft(const char *basename, const char *path, void *data)
640665
return 0;
641666
}
642667

643-
static int fsck_subdir(unsigned int nr, const char *path, void *progress)
668+
static int fsck_subdir(unsigned int nr, const char *path, void *data)
644669
{
670+
struct for_each_loose_cb *cb_data = data;
671+
struct progress *progress = cb_data->progress;
645672
display_progress(progress, nr + 1);
646673
return 0;
647674
}
648675

649676
static void fsck_object_dir(const char *path)
650677
{
651678
struct progress *progress = NULL;
679+
struct for_each_loose_cb cb_data = {
680+
.obj_type = STRBUF_INIT,
681+
.progress = progress,
682+
};
652683

653684
if (verbose)
654685
fprintf_ln(stderr, _("Checking object directory"));
@@ -657,9 +688,10 @@ static void fsck_object_dir(const char *path)
657688
progress = start_progress(_("Checking object directories"), 256);
658689

659690
for_each_loose_file_in_objdir(path, fsck_loose, fsck_cruft, fsck_subdir,
660-
progress);
691+
&cb_data);
661692
display_progress(progress, 256);
662693
stop_progress(&progress);
694+
strbuf_release(&cb_data.obj_type);
663695
}
664696

665697
static int fsck_head_link(const char *head_ref_name,

‎builtin/index-pack.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1415,7 +1415,7 @@ static void fix_unresolved_deltas(struct hashfile *f)
14151415

14161416
if (check_object_signature(the_repository, &d->oid,
14171417
data, size,
1418-
type_name(type)))
1418+
type_name(type), NULL))
14191419
die(_("local object %s is corrupt"), oid_to_hex(&d->oid));
14201420

14211421
/*

‎builtin/mktag.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ static int verify_object_in_tag(struct object_id *tagged_oid, int *tagged_type)
6262

6363
repl = lookup_replace_object(the_repository, tagged_oid);
6464
ret = check_object_signature(the_repository, repl,
65-
buffer, size, type_name(*tagged_type));
65+
buffer, size, type_name(*tagged_type),
66+
NULL);
6667
free(buffer);
6768

6869
return ret;

‎cache.h‎

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,11 +1268,50 @@ char *xdg_cache_home(const char *filename);
12681268

12691269
int git_open_cloexec(const char *name, int flags);
12701270
#define git_open(name) git_open_cloexec(name, O_RDONLY)
1271-
int unpack_loose_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz);
1272-
int parse_loose_header(const char *hdr, unsigned long *sizep);
1271+
1272+
/**
1273+
* unpack_loose_header() initializes the data stream needed to unpack
1274+
* a loose object header.
1275+
*
1276+
* Returns:
1277+
*
1278+
* - ULHR_OK on success
1279+
* - ULHR_BAD on error
1280+
* - ULHR_TOO_LONG if the header was too long
1281+
*
1282+
* It will only parse up to MAX_HEADER_LEN bytes unless an optional
1283+
* "hdrbuf" argument is non-NULL. This is intended for use with
1284+
* OBJECT_INFO_ALLOW_UNKNOWN_TYPE to extract the bad type for (error)
1285+
* reporting. The full header will be extracted to "hdrbuf" for use
1286+
* with parse_loose_header(), ULHR_TOO_LONG will still be returned
1287+
* from this function to indicate that the header was too long.
1288+
*/
1289+
enum unpack_loose_header_result {
1290+
ULHR_OK,
1291+
ULHR_BAD,
1292+
ULHR_TOO_LONG,
1293+
};
1294+
enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
1295+
unsigned char *map,
1296+
unsigned long mapsize,
1297+
void *buffer,
1298+
unsigned long bufsiz,
1299+
struct strbuf *hdrbuf);
1300+
1301+
/**
1302+
* parse_loose_header() parses the starting "<type> <len>\0" of an
1303+
* object. If it doesn't follow that format -1 is returned. To check
1304+
* the validity of the <type> populate the "typep" in the "struct
1305+
* object_info". It will be OBJ_BAD if the object type is unknown. The
1306+
* parsed <len> can be retrieved via "oi->sizep", and from there
1307+
* passed to unpack_loose_rest().
1308+
*/
1309+
struct object_info;
1310+
int parse_loose_header(const char *hdr, struct object_info *oi);
12731311

12741312
int check_object_signature(struct repository *r, const struct object_id *oid,
1275-
void *buf, unsigned long size, const char *type);
1313+
void *buf, unsigned long size, const char *type,
1314+
struct object_id *real_oidp);
12761315

12771316
int finalize_object_file(const char *tmpfile, const char *filename);
12781317

0 commit comments

Comments
 (0)