diff mbox series

[2/2] stash show: fix segfault with --{include,only}-untracked

Message ID 24de72b34de45980196ed6df8b64782887e94f36.1620850247.git.liu.denton@gmail.com (mailing list archive)
State Accepted
Commit 1ff595d218d9175eee1db753844044c2746d0515
Headers show
Series Fixes for dl/stash-show-untracked | expand

Commit Message

Denton Liu May 12, 2021, 8:16 p.m. UTC
When `git stash show --include-untracked` or
`git stash show --only-untracked` is run on a stash that doesn't include
an untracked entry, a segfault occurs. This happens because we do not
check whether the untracked entry is actually present and just attempt
to blindly dereference it.

Ensure that the untracked entry is present before actually attempting to
dereference it.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 builtin/stash.c                    |  8 ++++++--
 t/t3905-stash-include-untracked.sh | 15 +++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

Comments

Junio C Hamano May 12, 2021, 11:48 p.m. UTC | #1
Denton Liu <liu.denton@gmail.com> writes:

> When `git stash show --include-untracked` or
> `git stash show --only-untracked` is run on a stash that doesn't include
> an untracked entry, a segfault occurs. This happens because we do not
> check whether the untracked entry is actually present and just attempt
> to blindly dereference it.
>
> Ensure that the untracked entry is present before actually attempting to
> dereference it.

Makes sense.  Thanks.

>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  builtin/stash.c                    |  8 ++++++--
>  t/t3905-stash-include-untracked.sh | 15 +++++++++++++++
>  2 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/stash.c b/builtin/stash.c
> index 8922a1240c..82e4829d44 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -900,10 +900,14 @@ static int show_stash(int argc, const char **argv, const char *prefix)
>  		diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
>  		break;
>  	case UNTRACKED_ONLY:
> -		diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
> +		if (info.has_u)
> +			diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
>  		break;
>  	case UNTRACKED_INCLUDE:
> -		diff_include_untracked(&info, &rev.diffopt);
> +		if (info.has_u)
> +			diff_include_untracked(&info, &rev.diffopt);
> +		else
> +			diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
>  		break;
>  	}
>  	log_tree_diff_flush(&rev);
> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
> index 2e6796725b..1c9765928d 100755
> --- a/t/t3905-stash-include-untracked.sh
> +++ b/t/t3905-stash-include-untracked.sh
> @@ -405,4 +405,19 @@ test_expect_success 'stash show --include-untracked errors on duplicate files' '
>  	test_i18ngrep "worktree and untracked commit have duplicate entries: tracked" err
>  '
>  
> +test_expect_success 'stash show --{include,only}-untracked on stashes without untracked entries' '
> +	git reset --hard &&
> +	git clean -xf &&
> +	>tracked &&
> +	git add tracked &&
> +	git stash &&
> +
> +	git stash show >expect &&
> +	git stash show --include-untracked >actual &&
> +	test_cmp expect actual &&
> +
> +	git stash show --only-untracked >actual &&
> +	test_must_be_empty actual
> +'
> +
>  test_done
diff mbox series

Patch

diff --git a/builtin/stash.c b/builtin/stash.c
index 8922a1240c..82e4829d44 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -900,10 +900,14 @@  static int show_stash(int argc, const char **argv, const char *prefix)
 		diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
 		break;
 	case UNTRACKED_ONLY:
-		diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
+		if (info.has_u)
+			diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
 		break;
 	case UNTRACKED_INCLUDE:
-		diff_include_untracked(&info, &rev.diffopt);
+		if (info.has_u)
+			diff_include_untracked(&info, &rev.diffopt);
+		else
+			diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
 		break;
 	}
 	log_tree_diff_flush(&rev);
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index 2e6796725b..1c9765928d 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -405,4 +405,19 @@  test_expect_success 'stash show --include-untracked errors on duplicate files' '
 	test_i18ngrep "worktree and untracked commit have duplicate entries: tracked" err
 '
 
+test_expect_success 'stash show --{include,only}-untracked on stashes without untracked entries' '
+	git reset --hard &&
+	git clean -xf &&
+	>tracked &&
+	git add tracked &&
+	git stash &&
+
+	git stash show >expect &&
+	git stash show --include-untracked >actual &&
+	test_cmp expect actual &&
+
+	git stash show --only-untracked >actual &&
+	test_must_be_empty actual
+'
+
 test_done