{"thread":{"id":"55674","subject":"[PATCH 2/2] stash show: fix segfault with --{include,only}-untracked","startedAt":"2021-05-12T20:45:25Z","lastAt":"2021-05-12T23:54:21Z","messageCount":4,"participants":["Denton Liu","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"424374","messageId":"24de72b34de45980196ed6df8b64782887e94f36.1620850247.git.liu.denton@gmail.com","threadId":"55674","inReplyTo":"cover.1620850247.git.liu.denton@gmail.com","subject":"[PATCH 2/2] stash show: fix segfault with --{include,only}-untracked","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-05-12T20:16:13Z","receivedAt":"2021-05-12T20:45:25Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"When `git stash show --include-untracked` or\n`git stash show --only-untracked` is run on a stash that doesn't include\nan untracked entry, a segfault occurs. This happens because we do not\ncheck whether the untracked entry is actually present and just attempt\nto blindly dereference it.\n\nEnsure that the untracked entry is present before actually attempting to\ndereference it.\n\nSigned-off-by: Denton Liu \n---\n builtin/stash.c | 8 ++++++--\n t/t3905-stash-include-untracked.sh | 15 +++++++++++++++\n 2 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 8922a1240c..82e4829d44 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -900,10 +900,14 @@ static int show_stash(int argc, const char **argv, const char *prefix)\n \t\tdiff_tree_oid(&info.b_commit, &info.w_commit, \"\", &rev.diffopt);\n \t\tbreak;\n \tcase UNTRACKED_ONLY:\n-\t\tdiff_root_tree_oid(&info.u_tree, \"\", &rev.diffopt);\n+\t\tif (info.has_u)\n+\t\t\tdiff_root_tree_oid(&info.u_tree, \"\", &rev.diffopt);\n \t\tbreak;\n \tcase UNTRACKED_INCLUDE:\n-\t\tdiff_include_untracked(&info, &rev.diffopt);\n+\t\tif (info.has_u)\n+\t\t\tdiff_include_untracked(&info, &rev.diffopt);\n+\t\telse\n+\t\t\tdiff_tree_oid(&info.b_commit, &info.w_commit, \"\", &rev.diffopt);\n \t\tbreak;\n \t}\n \tlog_tree_diff_flush(&rev);\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex 2e6796725b..1c9765928d 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -405,4 +405,19 @@ test_expect_success 'stash show --include-untracked errors on duplicate files' '\n \ttest_i18ngrep \"worktree and untracked commit have duplicate entries: tracked\" err\n '\n \n+test_expect_success 'stash show --{include,only}-untracked on stashes without untracked entries' '\n+\tgit reset --hard &&\n+\tgit clean -xf &&\n+\t>tracked &&\n+\tgit add tracked &&\n+\tgit stash &&\n+\n+\tgit stash show >expect &&\n+\tgit stash show --include-untracked >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tgit stash show --only-untracked >actual &&\n+\ttest_must_be_empty actual\n+'\n+\n test_done\n-- \n2.31.1.751.gd2f1c929bd\n\n"},{"id":"424375","messageId":"cover.1620850247.git.liu.denton@gmail.com","threadId":"55674","inReplyTo":null,"subject":"[PATCH 0/2] Fixes for dl/stash-show-untracked","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-05-12T20:16:11Z","receivedAt":"2021-05-12T20:45:25Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"These follow-up patches for the topic should be merged before the\nnext release happens. In particular, the last patch fixes a potential\nsegfault that occurs.\n\nDenton Liu (2):\n t3905: correct test title\n stash show: fix segfault with --{include,only}-untracked\n\n builtin/stash.c | 8 ++++++--\n t/t3905-stash-include-untracked.sh | 17 ++++++++++++++++-\n 2 files changed, 22 insertions(+), 3 deletions(-)\n\n-- \n2.31.1.751.gd2f1c929bd\n\n"},{"id":"424376","messageId":"1f554261a57fd7e379d5d7fa81be871a55ed5ec5.1620850247.git.liu.denton@gmail.com","threadId":"55674","inReplyTo":"cover.1620850247.git.liu.denton@gmail.com","subject":"[PATCH 1/2] t3905: correct test title","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-05-12T20:16:12Z","receivedAt":"2021-05-12T20:45:26Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"We reference the non-existent option `git stash show --show-untracked`\nwhen we really meant `--only-untracked`. Correct the test title\naccordingly.\n\nSigned-off-by: Denton Liu \n---\n t/t3905-stash-include-untracked.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex b470db7ef7..2e6796725b 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -367,7 +367,7 @@ test_expect_success 'stash show --only-untracked only shows untracked files' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'stash show --no-include-untracked cancels --{include,show}-untracked' '\n+test_expect_success 'stash show --no-include-untracked cancels --{include,only}-untracked' '\n \tgit reset --hard &&\n \tgit clean -xf &&\n \t>untracked &&\n-- \n2.31.1.751.gd2f1c929bd\n\n"},{"id":"424404","messageId":"xmqq35urecns.fsf@gitster.g","threadId":"55674","inReplyTo":"24de72b34de45980196ed6df8b64782887e94f36.1620850247.git.liu.denton@gmail.com","subject":"Re: [PATCH 2/2] stash show: fix segfault with --{include,only}-untracked","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-12T23:48:55Z","receivedAt":"2021-05-12T23:54:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu writes:\n\n> When `git stash show --include-untracked` or\n> `git stash show --only-untracked` is run on a stash that doesn't include\n> an untracked entry, a segfault occurs. This happens because we do not\n> check whether the untracked entry is actually present and just attempt\n> to blindly dereference it.\n>\n> Ensure that the untracked entry is present before actually attempting to\n> dereference it.\n\nMakes sense. Thanks.\n\n>\n> Signed-off-by: Denton Liu \n> ---\n> builtin/stash.c | 8 ++++++--\n> t/t3905-stash-include-untracked.sh | 15 +++++++++++++++\n> 2 files changed, 21 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index 8922a1240c..82e4829d44 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -900,10 +900,14 @@ static int show_stash(int argc, const char **argv, const char *prefix)\n> \t\tdiff_tree_oid(&info.b_commit, &info.w_commit, \"\", &rev.diffopt);\n> \t\tbreak;\n> \tcase UNTRACKED_ONLY:\n> -\t\tdiff_root_tree_oid(&info.u_tree, \"\", &rev.diffopt);\n> +\t\tif (info.has_u)\n> +\t\t\tdiff_root_tree_oid(&info.u_tree, \"\", &rev.diffopt);\n> \t\tbreak;\n> \tcase UNTRACKED_INCLUDE:\n> -\t\tdiff_include_untracked(&info, &rev.diffopt);\n> +\t\tif (info.has_u)\n> +\t\t\tdiff_include_untracked(&info, &rev.diffopt);\n> +\t\telse\n> +\t\t\tdiff_tree_oid(&info.b_commit, &info.w_commit, \"\", &rev.diffopt);\n> \t\tbreak;\n> \t}\n> \tlog_tree_diff_flush(&rev);\n> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\n> index 2e6796725b..1c9765928d 100755\n> --- a/t/t3905-stash-include-untracked.sh\n> +++ b/t/t3905-stash-include-untracked.sh\n> @@ -405,4 +405,19 @@ test_expect_success 'stash show --include-untracked errors on duplicate files' '\n> \ttest_i18ngrep \"worktree and untracked commit have duplicate entries: tracked\" err\n> '\n> \n> +test_expect_success 'stash show --{include,only}-untracked on stashes without untracked entries' '\n> +\tgit reset --hard &&\n> +\tgit clean -xf &&\n> +\t>tracked &&\n> +\tgit add tracked &&\n> +\tgit stash &&\n> +\n> +\tgit stash show >expect &&\n> +\tgit stash show --include-untracked >actual &&\n> +\ttest_cmp expect actual &&\n> +\n> +\tgit stash show --only-untracked >actual &&\n> +\ttest_must_be_empty actual\n> +'\n> +\n> test_done\n"}]}