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

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

[Michael Montalbo via GitGitGadget]

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);
 
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
+	)
+'
+
 test_expect_success 'diff D F and diff F D' '
 	(
 		cd repo &&

base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
-- 
gitgitgadget

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

[Junio C Hamano]

"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