Re: [PATCH] diff: fix crash with --find-object outside repository

[Junio C Hamano <gitster@pobox.com>]

"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Michael Montalbo <mmontalbo@gmail.com>
>
> When "git diff --find-object=<oid>" is run outside a git repository,
> the option parsing callback eagerly resolves the OID via
> repo_get_oid(), which reaches get_main_ref_store() and hits a BUG()
> assertion because no repository has been set up.
>
> Check startup_info->have_repository before attempting to resolve the
> OID, and return a user-friendly error instead.
>
> Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
> ---
>     diff: fix crash with --find-object outside repository
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2057%2Fmmontalbo%2Ffix-find-object-crash-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2057/mmontalbo/fix-find-object-crash-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2057
>
>  diff.c                   |  2 ++
>  t/t4053-diff-no-index.sh | 10 ++++++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/diff.c b/diff.c
> index a68ddd2168..8f0f141be7 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -5215,6 +5215,8 @@ static int diff_opt_find_object(const struct option *option,
>  	struct object_id oid;
>  
>  	BUG_ON_OPT_NEG(unset);
> +	if (!startup_info->have_repository)
> +		return error(_("--find-object requires a git repository"));
>  	if (repo_get_oid(the_repository, arg, &oid))
>  		return error(_("unable to resolve '%s'"), arg);

Makes sense.

> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
> index 69599279e9..15076dfe0d 100755
> --- a/t/t4053-diff-no-index.sh
> +++ b/t/t4053-diff-no-index.sh
> @@ -76,6 +76,16 @@ test_expect_success 'git diff --no-index executed outside repo gives correct err
>  	)
>  '
>  
> +test_expect_success 'git diff --find-object outside repo fails gracefully' '
> +	(
> +		GIT_CEILING_DIRECTORIES=$TRASH_DIRECTORY/non &&
> +		export GIT_CEILING_DIRECTORIES &&
> +		cd non/git &&
> +		test_must_fail git diff --find-object=abc123 2>err &&
> +		test_grep "find-object requires a git repository" err
> +	)
> +'

Looks good, too.

Will queue.  Thanks.



>  test_expect_success 'diff D F and diff F D' '
>  	(
>  		cd repo &&
>
> base-commit: 67ad42147a7acc2af6074753ebd03d904476118f