Skip to content

Commit 41df0e3

Browse files
derrickstoleegitster
authored andcommitted
commit-graph: verify contents match checksum
The commit-graph file ends with a SHA1 hash of the previous contents. If a commit-graph file has errors but the checksum hash is correct, then we know that the problem is a bug in Git and not simply file corruption after-the-fact. Compute the checksum right away so it is the first error that appears, and make the message translatable since this error can be "corrected" by a user by simply deleting the file and recomputing. The rest of the errors are useful only to developers. Be sure to continue checking the rest of the file data if the checksum is wrong. This is important for our tests, as we break the checksum as we modify bytes of the commit-graph file. Signed-off-by: Derrick Stolee <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 437787a commit 41df0e3

File tree

2 files changed

+20
-2
lines changed

2 files changed

+20
-2
lines changed

commit-graph.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,7 @@ void write_commit_graph(const char *obj_dir,
833833
oids.nr = 0;
834834
}
835835

836+
#define VERIFY_COMMIT_GRAPH_ERROR_HASH 2
836837
static int verify_commit_graph_error;
837838

838839
static void graph_report(const char *fmt, ...)
@@ -852,8 +853,10 @@ static void graph_report(const char *fmt, ...)
852853
int verify_commit_graph(struct repository *r, struct commit_graph *g)
853854
{
854855
uint32_t i, cur_fanout_pos = 0;
855-
struct object_id prev_oid, cur_oid;
856+
struct object_id prev_oid, cur_oid, checksum;
856857
int generation_zero = 0;
858+
struct hashfile *f;
859+
int devnull;
857860

858861
if (!g) {
859862
graph_report("no commit-graph file loaded");
@@ -872,6 +875,15 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
872875
if (verify_commit_graph_error)
873876
return verify_commit_graph_error;
874877

878+
devnull = open("/dev/null", O_WRONLY);
879+
f = hashfd(devnull, NULL);
880+
hashwrite(f, g->data, g->data_len - g->hash_len);
881+
finalize_hashfile(f, checksum.hash, CSUM_CLOSE);
882+
if (hashcmp(checksum.hash, g->data + g->data_len - g->hash_len)) {
883+
graph_report(_("the commit-graph file has incorrect checksum and is likely corrupt"));
884+
verify_commit_graph_error = VERIFY_COMMIT_GRAPH_ERROR_HASH;
885+
}
886+
875887
for (i = 0; i < g->num_commits; i++) {
876888
struct commit *graph_commit;
877889

@@ -909,7 +921,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
909921
cur_fanout_pos++;
910922
}
911923

912-
if (verify_commit_graph_error)
924+
if (verify_commit_graph_error & ~VERIFY_COMMIT_GRAPH_ERROR_HASH)
913925
return verify_commit_graph_error;
914926

915927
for (i = 0; i < g->num_commits; i++) {

t/t5318-commit-graph.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ GRAPH_COMMIT_DATA_WIDTH=$(($HASH_LEN + 16))
279279
GRAPH_OCTOPUS_DATA_OFFSET=$(($GRAPH_COMMIT_DATA_OFFSET + \
280280
$GRAPH_COMMIT_DATA_WIDTH * $NUM_COMMITS))
281281
GRAPH_BYTE_OCTOPUS=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4))
282+
GRAPH_BYTE_FOOTER=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4 * $NUM_OCTOPUS_EDGES))
282283

283284
# usage: corrupt_graph_and_verify <position> <data> <string>
284285
# Manipulates the commit-graph file at the position
@@ -393,4 +394,9 @@ test_expect_success 'detect incorrect parent for octopus merge' '
393394
"invalid parent"
394395
'
395396

397+
test_expect_success 'detect invalid checksum hash' '
398+
corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \
399+
"incorrect checksum"
400+
'
401+
396402
test_done

0 commit comments

Comments
 (0)