Merge branch 'jk/commit-graph-verify-fix'

Various fixes to "git commit-graph verify".

* jk/commit-graph-verify-fix:
  commit-graph: report incomplete chains during verification
  commit-graph: tighten chain size check
  commit-graph: detect read errors when verifying graph chain
  t5324: harmonize sha1/sha256 graph chain corruption
  commit-graph: check mixed generation validation when loading chain file
  commit-graph: factor out chain opening function
This commit is contained in:
Junio C Hamano 2023-10-04 13:28:53 -07:00
commit c3c0020673
4 changed files with 158 additions and 55 deletions

View file

@ -69,10 +69,12 @@ static int graph_verify(int argc, const char **argv, const char *prefix)
struct commit_graph *graph = NULL; struct commit_graph *graph = NULL;
struct object_directory *odb = NULL; struct object_directory *odb = NULL;
char *graph_name; char *graph_name;
int open_ok; char *chain_name;
enum { OPENED_NONE, OPENED_GRAPH, OPENED_CHAIN } opened = OPENED_NONE;
int fd; int fd;
struct stat st; struct stat st;
int flags = 0; int flags = 0;
int incomplete_chain = 0;
int ret; int ret;
static struct option builtin_commit_graph_verify_options[] = { static struct option builtin_commit_graph_verify_options[] = {
@ -102,24 +104,39 @@ static int graph_verify(int argc, const char **argv, const char *prefix)
odb = find_odb(the_repository, opts.obj_dir); odb = find_odb(the_repository, opts.obj_dir);
graph_name = get_commit_graph_filename(odb); graph_name = get_commit_graph_filename(odb);
open_ok = open_commit_graph(graph_name, &fd, &st); chain_name = get_commit_graph_chain_filename(odb);
if (!open_ok && errno != ENOENT) if (open_commit_graph(graph_name, &fd, &st))
opened = OPENED_GRAPH;
else if (errno != ENOENT)
die_errno(_("Could not open commit-graph '%s'"), graph_name); die_errno(_("Could not open commit-graph '%s'"), graph_name);
else if (open_commit_graph_chain(chain_name, &fd, &st))
opened = OPENED_CHAIN;
else if (errno != ENOENT)
die_errno(_("could not open commit-graph chain '%s'"), chain_name);
FREE_AND_NULL(graph_name); FREE_AND_NULL(graph_name);
FREE_AND_NULL(chain_name);
FREE_AND_NULL(options); FREE_AND_NULL(options);
if (open_ok) if (opened == OPENED_NONE)
return 0;
else if (opened == OPENED_GRAPH)
graph = load_commit_graph_one_fd_st(the_repository, fd, &st, odb); graph = load_commit_graph_one_fd_st(the_repository, fd, &st, odb);
else else
graph = read_commit_graph_one(the_repository, odb); graph = load_commit_graph_chain_fd_st(the_repository, fd, &st,
&incomplete_chain);
/* Return failure if open_ok predicted success */
if (!graph) if (!graph)
return !!open_ok; return 1;
ret = verify_commit_graph(the_repository, graph, flags); ret = verify_commit_graph(the_repository, graph, flags);
free_commit_graph(graph); free_commit_graph(graph);
if (incomplete_chain) {
error("one or more commit-graph chain files could not be loaded");
ret |= 1;
}
return ret; return ret;
} }

View file

@ -473,6 +473,31 @@ static struct commit_graph *load_commit_graph_v1(struct repository *r,
return g; return g;
} }
/*
* returns 1 if and only if all graphs in the chain have
* corrected commit dates stored in the generation_data chunk.
*/
static int validate_mixed_generation_chain(struct commit_graph *g)
{
int read_generation_data = 1;
struct commit_graph *p = g;
while (read_generation_data && p) {
read_generation_data = p->read_generation_data;
p = p->base_graph;
}
if (read_generation_data)
return 1;
while (g) {
g->read_generation_data = 0;
g = g->base_graph;
}
return 0;
}
static int add_graph_to_chain(struct commit_graph *g, static int add_graph_to_chain(struct commit_graph *g,
struct commit_graph *chain, struct commit_graph *chain,
struct object_id *oids, struct object_id *oids,
@ -513,31 +538,41 @@ static int add_graph_to_chain(struct commit_graph *g,
return 1; return 1;
} }
static struct commit_graph *load_commit_graph_chain(struct repository *r, int open_commit_graph_chain(const char *chain_file,
struct object_directory *odb) int *fd, struct stat *st)
{
*fd = git_open(chain_file);
if (*fd < 0)
return 0;
if (fstat(*fd, st)) {
close(*fd);
return 0;
}
if (st->st_size < the_hash_algo->hexsz) {
close(*fd);
if (!st->st_size) {
/* treat empty files the same as missing */
errno = ENOENT;
} else {
warning("commit-graph chain file too small");
errno = EINVAL;
}
return 0;
}
return 1;
}
struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r,
int fd, struct stat *st,
int *incomplete_chain)
{ {
struct commit_graph *graph_chain = NULL; struct commit_graph *graph_chain = NULL;
struct strbuf line = STRBUF_INIT; struct strbuf line = STRBUF_INIT;
struct stat st;
struct object_id *oids; struct object_id *oids;
int i = 0, valid = 1, count; int i = 0, valid = 1, count;
char *chain_name = get_commit_graph_chain_filename(odb); FILE *fp = xfdopen(fd, "r");
FILE *fp;
int stat_res;
fp = fopen(chain_name, "r"); count = st->st_size / (the_hash_algo->hexsz + 1);
stat_res = stat(chain_name, &st);
free(chain_name);
if (!fp)
return NULL;
if (stat_res ||
st.st_size <= the_hash_algo->hexsz) {
fclose(fp);
return NULL;
}
count = st.st_size / (the_hash_algo->hexsz + 1);
CALLOC_ARRAY(oids, count); CALLOC_ARRAY(oids, count);
prepare_alt_odb(r); prepare_alt_odb(r);
@ -578,36 +613,32 @@ static struct commit_graph *load_commit_graph_chain(struct repository *r,
} }
} }
validate_mixed_generation_chain(graph_chain);
free(oids); free(oids);
fclose(fp); fclose(fp);
strbuf_release(&line); strbuf_release(&line);
*incomplete_chain = !valid;
return graph_chain; return graph_chain;
} }
/* static struct commit_graph *load_commit_graph_chain(struct repository *r,
* returns 1 if and only if all graphs in the chain have struct object_directory *odb)
* corrected commit dates stored in the generation_data chunk.
*/
static int validate_mixed_generation_chain(struct commit_graph *g)
{ {
int read_generation_data = 1; char *chain_file = get_commit_graph_chain_filename(odb);
struct commit_graph *p = g; struct stat st;
int fd;
struct commit_graph *g = NULL;
while (read_generation_data && p) { if (open_commit_graph_chain(chain_file, &fd, &st)) {
read_generation_data = p->read_generation_data; int incomplete;
p = p->base_graph; /* ownership of fd is taken over by load function */
g = load_commit_graph_chain_fd_st(r, fd, &st, &incomplete);
} }
if (read_generation_data) free(chain_file);
return 1; return g;
while (g) {
g->read_generation_data = 0;
g = g->base_graph;
}
return 0;
} }
struct commit_graph *read_commit_graph_one(struct repository *r, struct commit_graph *read_commit_graph_one(struct repository *r,
@ -618,8 +649,6 @@ struct commit_graph *read_commit_graph_one(struct repository *r,
if (!g) if (!g)
g = load_commit_graph_chain(r, odb); g = load_commit_graph_chain(r, odb);
validate_mixed_generation_chain(g);
return g; return g;
} }

View file

@ -26,6 +26,7 @@ struct string_list;
char *get_commit_graph_filename(struct object_directory *odb); char *get_commit_graph_filename(struct object_directory *odb);
char *get_commit_graph_chain_filename(struct object_directory *odb); char *get_commit_graph_chain_filename(struct object_directory *odb);
int open_commit_graph(const char *graph_file, int *fd, struct stat *st); int open_commit_graph(const char *graph_file, int *fd, struct stat *st);
int open_commit_graph_chain(const char *chain_file, int *fd, struct stat *st);
/* /*
* Given a commit struct, try to fill the commit struct info, including: * Given a commit struct, try to fill the commit struct info, including:
@ -105,6 +106,9 @@ struct commit_graph {
struct commit_graph *load_commit_graph_one_fd_st(struct repository *r, struct commit_graph *load_commit_graph_one_fd_st(struct repository *r,
int fd, struct stat *st, int fd, struct stat *st,
struct object_directory *odb); struct object_directory *odb);
struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r,
int fd, struct stat *st,
int *incomplete_chain);
struct commit_graph *read_commit_graph_one(struct repository *r, struct commit_graph *read_commit_graph_one(struct repository *r,
struct object_directory *odb); struct object_directory *odb);

View file

@ -285,6 +285,32 @@ test_expect_success 'verify hashes along chain, even in shallow' '
) )
' '
test_expect_success 'verify notices chain slice which is bogus (base)' '
git clone --no-hardlinks . verify-chain-bogus-base &&
(
cd verify-chain-bogus-base &&
git commit-graph verify &&
base_file=$graphdir/graph-$(sed -n 1p $graphdir/commit-graph-chain).graph &&
echo "garbage" >$base_file &&
test_must_fail git commit-graph verify 2>test_err &&
grep -v "^+" test_err >err &&
grep "commit-graph file is too small" err
)
'
test_expect_success 'verify notices chain slice which is bogus (tip)' '
git clone --no-hardlinks . verify-chain-bogus-tip &&
(
cd verify-chain-bogus-tip &&
git commit-graph verify &&
tip_file=$graphdir/graph-$(sed -n 2p $graphdir/commit-graph-chain).graph &&
echo "garbage" >$tip_file &&
test_must_fail git commit-graph verify 2>test_err &&
grep -v "^+" test_err >err &&
grep "commit-graph file is too small" err
)
'
test_expect_success 'verify --shallow does not check base contents' ' test_expect_success 'verify --shallow does not check base contents' '
git clone --no-hardlinks . verify-shallow && git clone --no-hardlinks . verify-shallow &&
( (
@ -306,27 +332,54 @@ test_expect_success 'warn on base graph chunk incorrect' '
git commit-graph verify && git commit-graph verify &&
base_file=$graphdir/graph-$(tail -n 1 $graphdir/commit-graph-chain).graph && base_file=$graphdir/graph-$(tail -n 1 $graphdir/commit-graph-chain).graph &&
corrupt_file "$base_file" $(test_oid base) "\01" && corrupt_file "$base_file" $(test_oid base) "\01" &&
git commit-graph verify --shallow 2>test_err && test_must_fail git commit-graph verify --shallow 2>test_err &&
grep -v "^+" test_err >err && grep -v "^+" test_err >err &&
test_i18ngrep "commit-graph chain does not match" err test_i18ngrep "commit-graph chain does not match" err
) )
' '
test_expect_success 'verify after commit-graph-chain corruption' ' test_expect_success 'verify after commit-graph-chain corruption (base)' '
git clone --no-hardlinks . verify-chain && git clone --no-hardlinks . verify-chain-base &&
( (
cd verify-chain && cd verify-chain-base &&
corrupt_file "$graphdir/commit-graph-chain" 60 "G" && corrupt_file "$graphdir/commit-graph-chain" 30 "G" &&
git commit-graph verify 2>test_err && test_must_fail git commit-graph verify 2>test_err &&
grep -v "^+" test_err >err && grep -v "^+" test_err >err &&
test_i18ngrep "invalid commit-graph chain" err && test_i18ngrep "invalid commit-graph chain" err &&
corrupt_file "$graphdir/commit-graph-chain" 60 "A" && corrupt_file "$graphdir/commit-graph-chain" 30 "A" &&
git commit-graph verify 2>test_err && test_must_fail git commit-graph verify 2>test_err &&
grep -v "^+" test_err >err && grep -v "^+" test_err >err &&
test_i18ngrep "unable to find all commit-graph files" err test_i18ngrep "unable to find all commit-graph files" err
) )
' '
test_expect_success 'verify after commit-graph-chain corruption (tip)' '
git clone --no-hardlinks . verify-chain-tip &&
(
cd verify-chain-tip &&
corrupt_file "$graphdir/commit-graph-chain" 70 "G" &&
test_must_fail git commit-graph verify 2>test_err &&
grep -v "^+" test_err >err &&
test_i18ngrep "invalid commit-graph chain" err &&
corrupt_file "$graphdir/commit-graph-chain" 70 "A" &&
test_must_fail git commit-graph verify 2>test_err &&
grep -v "^+" test_err >err &&
test_i18ngrep "unable to find all commit-graph files" err
)
'
test_expect_success 'verify notices too-short chain file' '
git clone --no-hardlinks . verify-chain-short &&
(
cd verify-chain-short &&
git commit-graph verify &&
echo "garbage" >$graphdir/commit-graph-chain &&
test_must_fail git commit-graph verify 2>test_err &&
grep -v "^+" test_err >err &&
grep "commit-graph chain file too small" err
)
'
test_expect_success 'verify across alternates' ' test_expect_success 'verify across alternates' '
git clone --no-hardlinks . verify-alt && git clone --no-hardlinks . verify-alt &&
( (