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 <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/commit-graph.c b/commit-graph.c
index 3ac28b5eb5c63fd081424fa8354ca15d3dc58cc8..aca9e5e2dc994fdd4febce79982efd323f6558db 100644 (file)
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -833,6 +833,7 @@ void write_commit_graph(const char *obj_dir,
        oids.nr = 0;
 }
 
+#define VERIFY_COMMIT_GRAPH_ERROR_HASH 2
 static int verify_commit_graph_error;
 
 static void graph_report(const char *fmt, ...)
@@ -852,8 +853,10 @@ static void graph_report(const char *fmt, ...)
 int verify_commit_graph(struct repository *r, struct commit_graph *g)
 {
        uint32_t i, cur_fanout_pos = 0;
-       struct object_id prev_oid, cur_oid;
+       struct object_id prev_oid, cur_oid, checksum;
        int generation_zero = 0;
+       struct hashfile *f;
+       int devnull;
 
        if (!g) {
                graph_report("no commit-graph file loaded");
@@ -872,6 +875,15 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
        if (verify_commit_graph_error)
                return verify_commit_graph_error;
 
+       devnull = open("/dev/null", O_WRONLY);
+       f = hashfd(devnull, NULL);
+       hashwrite(f, g->data, g->data_len - g->hash_len);
+       finalize_hashfile(f, checksum.hash, CSUM_CLOSE);
+       if (hashcmp(checksum.hash, g->data + g->data_len - g->hash_len)) {
+               graph_report(_("the commit-graph file has incorrect checksum and is likely corrupt"));
+               verify_commit_graph_error = VERIFY_COMMIT_GRAPH_ERROR_HASH;
+       }
+
        for (i = 0; i < g->num_commits; i++) {
                struct commit *graph_commit;
 
@@ -909,7 +921,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
                cur_fanout_pos++;
        }
 
-       if (verify_commit_graph_error)
+       if (verify_commit_graph_error & ~VERIFY_COMMIT_GRAPH_ERROR_HASH)
                return verify_commit_graph_error;
 
        for (i = 0; i < g->num_commits; i++) {

diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index a0cf1f66de64eb8941ce485ec7c654c33c63bb56..fed05e2f12a23511aabf85318036d70cdf903c33 100755 (executable)
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -279,6 +279,7 @@ GRAPH_COMMIT_DATA_WIDTH=$(($HASH_LEN + 16))
 GRAPH_OCTOPUS_DATA_OFFSET=$(($GRAPH_COMMIT_DATA_OFFSET + \
                             $GRAPH_COMMIT_DATA_WIDTH * $NUM_COMMITS))
 GRAPH_BYTE_OCTOPUS=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4))
+GRAPH_BYTE_FOOTER=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4 * $NUM_OCTOPUS_EDGES))
 
 # usage: corrupt_graph_and_verify <position> <data> <string>
 # Manipulates the commit-graph file at the position
@@ -393,4 +394,9 @@ test_expect_success 'detect incorrect parent for octopus merge' '
                "invalid parent"
 '
 
+test_expect_success 'detect invalid checksum hash' '
+       corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \
+               "incorrect checksum"
+'
+
 test_done