On Thu, Apr 01 2021, Jeff King wrote:
> 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@xxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Jeff King <peff@xxxxxxxx>
+ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
## 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
