[PATCH v2 0/5] mktag tests & fix for-each-ref segfault
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Apr 1, 2021, 13:56 UTC
- Message-ID
- <cover-0.6-00000000000-20210401T135419Z-avarab@gmail.com>
- In-Reply-To
- <YGWFGMdGcKeaqCQF@coredump.intra.peff.net>
On Thu, Apr 01 2021, Jeff King wrote:
Show 18 quoted lines
> On Thu, Apr 01, 2021 at 03:54:56AM -0400, Jeff King wrote: > >> On Wed, Mar 31, 2021 at 10:46:22PM +0200, Ævar Arnfjörð Bjarmason wrote: >> >> > > Neither of those types is the correct one. And the segfault is just a >> > > bonus! :) >> > > >> > > I'd expect similar cases with parsing commit parents and tree pointers. >> > > And probably tree entries whose modes are wrong. >> > >> > So the segfault happens without my patches, >> >> Yeah, sorry if that was unclear. It is definitely a pre-existing bug. > > Here's a patch to fix it. This is mostly orthogonal to your patch > series. It happens to use a similar recipe to reproduce, but that is not > the only way to do it, and the fix and the test shouldn't conflict > textually or semantically.
Here's a proposed v2. We test the same case, but I thought it made sense to test this more exhaustively.
The v1 will also leave t6300 in a bad state for whoever adds the next test, trivial to fix with a test_create_repo, but this seems better.
Jeff King (1): ref-filter: fix NULL check for parse object failure
Ævar Arnfjörð Bjarmason (4): mktag tests: parse out options in helper mktag tests: invert --no-strict test mktag tests: do fsck on failure mktag tests: test for maybe segfaulting for-each-ref
ref-filter.c | 2 +- t/t3800-mktag.sh | 90 +++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 75 insertions(+), 17 deletions(-)
Range-diff:
-: ----------- > 1: 45e0f100613 mktag tests: parse out options in helper
-: ----------- > 2: dd71740447d mktag tests: invert --no-strict test
-: ----------- > 3: 688d7456843 mktag tests: do fsck on failure
-: ----------- > 4: 403024b1cca mktag tests: test for maybe segfaulting for-each-ref
1: 9358541ce1f ! 5: 2ffe8f9fe3c ref-filter: fix NULL check for parse object failure
@@ Commit message
There are many ways a parse could fail, but most of them are hard to set
up in the tests (it's easy to make a bogus object, but update-ref will
- refuse to point to it). The test here uses a tag which points to a wrong
- object type. A parse of just the broken tag object will succeed, but
- seeing both tag objects in the same process will lead to a parse error
- (since we'll see the pointed-to object as both types).
+ refuse to point to it).
+
+ A minimal stand-alone test can be found at, but let's use the newly
+ amended t3800-mktag.sh tests to test these cases exhaustively on all
+ sorts of bad tags.
+
+ 1. http://lore.kernel.org/git/YGWFGMdGcKeaqCQF@coredump.intra.peff.net
Signed-off-by: Jeff King <peff@peff.net>
+ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## ref-filter.c ##
@@ ref-filter.c: static int get_object(struct ref_array_item *ref, int deref, struct object **obj
@@ ref-filter.c: static int get_object(struct ref_array_item *ref, int deref, struc
free(oi->content);
return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
- ## t/t6300-for-each-ref.sh ##
-@@ t/t6300-for-each-ref.sh: test_expect_success 'for-each-ref --ignore-case works on multiple sort keys' '
- test_cmp expect actual
- '
+ ## t/t3800-mktag.sh ##
+@@ t/t3800-mktag.sh: check_verify_failure () {
+ git -C bad-tag for-each-ref "$tag_ref" >actual &&
+ test_cmp expected actual &&
+ # segfaults!
+- ! git -C bad-tag for-each-ref --format="%(*objectname)"
++ test_must_fail git -C bad-tag for-each-ref --format="%(*objectname)"
+ '
+ }
-+test_expect_success 'for-each-ref reports broken tags' '
-+ git tag -m "good tag" broken-tag-good HEAD &&
-+ git cat-file tag broken-tag-good >good &&
-+ sed s/commit/blob/ <good >bad &&
-+ bad=$(git hash-object -w -t tag bad) &&
-+ git update-ref refs/tags/broken-tag-bad $bad &&
-+ test_must_fail git for-each-ref --format="%(*objectname)" \
-+ refs/tags/broken-tag-*
-+'
-+
- test_done-- 2.31.1.474.g72d45d12706