@matheusbsbs / matheus-bagliano / commits / 7754a565e2

commit-graph: tighten chain size check

When we open a commit-graph-chain file, if it's smaller than a single entry, we just quietly treat that as ENOENT. That make some sense if the file is truly zero bytes, but it means that "commit-graph verify" will quietly ignore a file that contains garbage if that garbage happens to be short. Instead, let's only simulate ENOENT when the file is truly empty, and otherwise return EINVAL. The normal graph-loading routines don't care, but "commit-graph verify" will notice and complain about the difference. It's not entirely clear to me that the 0-is-ENOENT case actually happens in real life, so we could perhaps just eliminate this special-case altogether. But this is how we've always behaved, so I'm preserving it in the name of backwards compatibility (though again, it really only matters for "verify", as the regular routines are happy to load what they can). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 28, 2023 at 00:39 UTC 7754a565e2e78e4163dbf597bba5fc729cc3bbc7

2 files changed +20 -2

commit-graph.c

+8 -2

@@ -548,9 +548,15 @@ int open_commit_graph_chain(const char *chain_file,

548 close(*fd);

549 return 0;

550 }

551 - if (st->st_size <= the_hash_algo->hexsz) {

551 + if (st->st_size < the_hash_algo->hexsz) {

552 close(*fd);

553 - errno = ENOENT;

553 + if (!st->st_size) {

554 + /* treat empty files the same as missing */

555 + errno = ENOENT;

556 + } else {

557 + warning("commit-graph chain file too small");

558 + errno = EINVAL;

559 + }

560 return 0;

561 }

562 return 1;

t/t5324-split-commit-graph.sh

+12

@@ -342,6 +342,18 @@ test_expect_success 'verify after commit-graph-chain corruption (tip)' '

342 )

343 '

344

345 +test_expect_success 'verify notices too-short chain file' '

346 + git clone --no-hardlinks . verify-chain-short &&

347 + (

348 + cd verify-chain-short &&

349 + git commit-graph verify &&

350 + echo "garbage" >$graphdir/commit-graph-chain &&

351 + test_must_fail git commit-graph verify 2>test_err &&

352 + grep -v "^+" test_err >err &&

353 + grep "commit-graph chain file too small" err

354 + )

355 +'

356 +

357 test_expect_success 'verify across alternates' '

358 git clone --no-hardlinks . verify-alt &&

359 (