commit-graph: verify parent list

The commit-graph file stores parents in a two-column portion of the
commit data chunk. If there is only one parent, then the second column
stores 0xFFFFFFFF to indicate no second parent.

The 'verify' subcommand checks the parent list for the commit loaded
from the commit-graph and the one parsed from the object database. Test
these checks for corrupt parents, too many parents, and wrong parents.

Add a boundary check to insert_parent_or_die() for when the parent
position value is out of range.

The octopus merge will be tested in a later commit.

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 17624b90d763cf2044ce373226c84c3062e82851..a79b06f37a674ff800393bbebf9aac682a1015b9 100644 (file)
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -244,6 +244,9 @@ static struct commit_list **insert_parent_or_die(struct commit_graph *g,
        struct commit *c;
        struct object_id oid;
 
        struct commit *c;
        struct object_id oid;
 

+       if (pos >= g->num_commits)
+               die("invalid parent position %"PRIu64, pos);
+

        hashcpy(oid.hash, g->chunk_oid_lookup + g->hash_len * pos);
        c = lookup_commit(&oid);
        if (!c)
        hashcpy(oid.hash, g->chunk_oid_lookup + g->hash_len * pos);
        c = lookup_commit(&oid);
        if (!c)


@@ -907,6 +910,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
 
        for (i = 0; i < g->num_commits; i++) {
                struct commit *graph_commit, *odb_commit;
 
        for (i = 0; i < g->num_commits; i++) {
                struct commit *graph_commit, *odb_commit;

+               struct commit_list *graph_parents, *odb_parents;

 
                hashcpy(cur_oid.hash, g->chunk_oid_lookup + g->hash_len * i);
 
 
                hashcpy(cur_oid.hash, g->chunk_oid_lookup + g->hash_len * i);
 


@@ -924,6 +928,30 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
                                     oid_to_hex(&cur_oid),
                                     oid_to_hex(get_commit_tree_oid(graph_commit)),
                                     oid_to_hex(get_commit_tree_oid(odb_commit)));
                                     oid_to_hex(&cur_oid),
                                     oid_to_hex(get_commit_tree_oid(graph_commit)),
                                     oid_to_hex(get_commit_tree_oid(odb_commit)));

+
+               graph_parents = graph_commit->parents;
+               odb_parents = odb_commit->parents;
+
+               while (graph_parents) {
+                       if (odb_parents == NULL) {
+                               graph_report("commit-graph parent list for commit %s is too long",
+                                            oid_to_hex(&cur_oid));
+                               break;
+                       }
+
+                       if (oidcmp(&graph_parents->item->object.oid, &odb_parents->item->object.oid))
+                               graph_report("commit-graph parent for %s is %s != %s",
+                                            oid_to_hex(&cur_oid),
+                                            oid_to_hex(&graph_parents->item->object.oid),
+                                            oid_to_hex(&odb_parents->item->object.oid));
+
+                       graph_parents = graph_parents->next;
+                       odb_parents = odb_parents->next;
+               }
+
+               if (odb_parents != NULL)
+                       graph_report("commit-graph parent list for commit %s terminates early",
+                                    oid_to_hex(&cur_oid));

        }
 
        return verify_commit_graph_error;
        }
 
        return verify_commit_graph_error;

diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index f258c6d5d08fa231550f088e7468d0cc375f45bb..b41c8f4d9bdec73bcee6a150cefb3c4060ebb97a 100755 (executable)
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -269,6 +269,9 @@ GRAPH_BYTE_OID_LOOKUP_ORDER=$(($GRAPH_OID_LOOKUP_OFFSET + $HASH_LEN * 8))
 GRAPH_BYTE_OID_LOOKUP_MISSING=$(($GRAPH_OID_LOOKUP_OFFSET + $HASH_LEN * 4 + 10))
 GRAPH_COMMIT_DATA_OFFSET=$(($GRAPH_OID_LOOKUP_OFFSET + $HASH_LEN * $NUM_COMMITS))
 GRAPH_BYTE_COMMIT_TREE=$GRAPH_COMMIT_DATA_OFFSET
 GRAPH_BYTE_OID_LOOKUP_MISSING=$(($GRAPH_OID_LOOKUP_OFFSET + $HASH_LEN * 4 + 10))
 GRAPH_COMMIT_DATA_OFFSET=$(($GRAPH_OID_LOOKUP_OFFSET + $HASH_LEN * $NUM_COMMITS))
 GRAPH_BYTE_COMMIT_TREE=$GRAPH_COMMIT_DATA_OFFSET

+GRAPH_BYTE_COMMIT_PARENT=$(($GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN))
+GRAPH_BYTE_COMMIT_EXTRA_PARENT=$(($GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN + 4))
+GRAPH_BYTE_COMMIT_WRONG_PARENT=$(($GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN + 3))

 
 # usage: corrupt_graph_and_verify <position> <data> <string>
 # Manipulates the commit-graph file at the position
 
 # usage: corrupt_graph_and_verify <position> <data> <string>
 # Manipulates the commit-graph file at the position


@@ -348,4 +351,19 @@ test_expect_success 'detect incorrect tree OID' '
                "root tree OID for commit"
 '
 
                "root tree OID for commit"
 '
 

+test_expect_success 'detect incorrect parent int-id' '
+       corrupt_graph_and_verify $GRAPH_BYTE_COMMIT_PARENT "\01" \
+               "invalid parent"
+'
+
+test_expect_success 'detect extra parent int-id' '
+       corrupt_graph_and_verify $GRAPH_BYTE_COMMIT_EXTRA_PARENT "\00" \
+               "is too long"
+'
+
+test_expect_success 'detect wrong parent' '
+       corrupt_graph_and_verify $GRAPH_BYTE_COMMIT_WRONG_PARENT "\01" \
+               "commit-graph parent for"
+'
+

 test_done
 test_done