Re: [PATCH] ref-filter: don't look for objects when outside of a repository
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 24, 2018, 16:15 UTC
- Message-ID
- <xmqqzhw6swhf.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20180922141145.10558-1-szeder.dev@gmail.com>
SZEDER Gábor <szeder.dev@gmail.com> writes:
Show 20 quoted lines
> The command 'git ls-remote --sort=authordate <remote>' segfaults when > run outside of a repository, ever since the introduction of its > '--sort' option in 1fb20dfd8e (ls-remote: create '--sort' option, > 2018-04-09). > > While in general the 'git ls-remote' command can be run outside of a > repository just fine, its '--sort=<key>' option with certain keys does > require access to the referenced objects. This sorting is implemented > using the generic ref-filter sorting facility, which already handles > missing objects gracefully with the appropriate 'missing object > deadbeef for HEAD' message. However, being generic means that it > checks replace refs while trying to retrieve an object, and while > doing so it accesses the 'git_replace_ref_base' variable, which has > not been initialized and is still a NULL pointer when outside of a > repository, thus causing the segfault. > > Make ref-filter more careful and only attempt to retrieve an object > when we are in a repository. Also add a test to ensure that 'git > ls-remote --sort' fails gracefully when executed outside of a > repository.
OK. So by forcing get_object() return an error, we do the same to populate_value() which in turn will make get_ref_atgom_value return an error and cmp_ref_sorting() will notice and die.
I think that is the best we could do.
Show 41 quoted lines
>
> Reported-by: H.Merijn Brand <h.m.brand@xs4all.nl>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>
> I'm not quite sure that this is the best place to add this check...
> but hey, it's a Saturday afternoon after all ;)
>
> ref-filter.c | 3 ++-
> t/t5512-ls-remote.sh | 6 ++++++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index e1bcb4ca8a..3555bc29e7 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1473,7 +1473,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
> oi->info.sizep = &oi->size;
> oi->info.typep = &oi->type;
> }
> - if (oid_object_info_extended(the_repository, &oi->oid, &oi->info,
> + if (!have_git_dir() ||
> + oid_object_info_extended(the_repository, &oi->oid, &oi->info,
> OBJECT_INFO_LOOKUP_REPLACE))
> return strbuf_addf_ret(err, -1, _("missing object %s for %s"),
> oid_to_hex(&oi->oid), ref->refname);
> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
> index bc5703ff9b..7dd081da01 100755
> --- a/t/t5512-ls-remote.sh
> +++ b/t/t5512-ls-remote.sh
> @@ -302,4 +302,10 @@ test_expect_success 'ls-remote works outside repository' '
> nongit git ls-remote dst.git
> '
>
> +test_expect_success 'ls-remote --sort fails gracefully outside repository' '
> + # Use a sort key that requires access to the referenced objects.
> + nongit test_must_fail git ls-remote --sort=authordate "$TRASH_DIRECTORY" 2>err &&
> + test_i18ngrep "^fatal: missing object" err
> +'
> +
> test_done