{"thread":{"id":"36932","subject":"[PATCH] pretty: avoid reading past end-of-string with \"%G\"","startedAt":"2014-06-16T20:13:11Z","lastAt":"2014-06-17T00:07:07Z","messageCount":10,"participants":["Jeff King","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"244348","messageId":"20140616201311.GA26829@sigill.intra.peff.net","threadId":"36932","inReplyTo":null,"subject":"[PATCH] pretty: avoid reading past end-of-string with \"%G\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-06-16T20:13:11Z","receivedAt":"2014-06-16T20:13:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If the user asks for --format=%G with nothing else, we\ncorrectly realize that \"%G\" is not a valid placeholder (it\nshould be \"%G?\", \"%GK\", etc). But we still tell the\nstrbuf_expand code that we consumed 2 characters, causing it\nto jump over the trailing NUL and output garbage.\n\nThis also fixes the case where \"%GX\" would be consumed (and\nproduce no output). In other cases, we pass unrecognized\nplaceholders through to the final string.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nNoticed while experimenting with \"%G\" placeholders in the nearby thread.\n\nIt doesn't look like we have any tests of \"%G*\" and friends at all. :(\n\n pretty.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/pretty.c b/pretty.c\nindex e1e2cad..70d8776 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1267,6 +1267,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\tif (c->signature_check.key)\n \t\t\t\tstrbuf_addstr(sb, c->signature_check.key);\n \t\t\tbreak;\n+\t\tdefault:\n+\t\t\treturn 0;\n \t\t}\n \t\treturn 2;\n \t}\n-- \n2.0.0.566.gfe3e6b2\n"},{"id":"244352","messageId":"20140616202611.GA29716@sigill.intra.peff.net","threadId":"36932","inReplyTo":"20140616201311.GA26829@sigill.intra.peff.net","subject":"[PATCH] t7510: check %G* pretty-format output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-06-16T20:26:11Z","receivedAt":"2014-06-16T20:26:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 16, 2014 at 04:13:11PM -0400, Jeff King wrote:\n\n> It doesn't look like we have any tests of \"%G*\" and friends at all. :(\n\nMaybe we can add this:\n\n-- >8 --\nSubject: t7510: check %G* pretty-format output\n\nWe do not check these along with the other pretty-format\nplaceholders in t6006, because we need signed commits to\nmake them interesting. t7510 has such commits, and can\neasily exercise them in addition to the regular\n--show-signature code path.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI explicitly avoided \"%GG\" here, as its exact format is dependent on\ngpg (and the current date). I don't know that it is worth the\ncomplexity to test, as the interesting parts are already parsed from it\nand exposed in the other placeholders.\n\n t/t7510-signed-commit.sh | 36 ++++++++++++++++++++++++++++++++++++\n 1 file changed, 36 insertions(+)\n\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 5ddac1a..2f96937 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -76,6 +76,42 @@ test_expect_success GPG 'detect fudged signature' '\n \t! grep \"Good signature from\" actual1\n '\n \n+test_expect_success GPG 'show good signature with custom format' '\n+\tcat >expect <<-\\EOF\n+\tG\n+\t13B6F51ECDDE430D\n+\tC O Mitter <committer@example.com>\n+\tEOF\n+\tgit log -1 --format=\"%G?%n%GK%n%GS\" master >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show bad signature with custom format' '\n+\tcat >expect <<-\\EOF\n+\tB\n+\t13B6F51ECDDE430D\n+\tC O Mitter <committer@example.com>\n+\tEOF\n+\tgit log -1 --format=\"%G?%n%GK%n%GS\" $(cat forged1.commit) >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show lack of signature with custom format' '\n+\tcat >expect <<-\\EOF\n+\tN\n+\n+\n+\tEOF\n+\tgit log -1 --format=\"%G?%n%GK%n%GS\" seventh-unsigned >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'unused %G placeholders are passed through' '\n+\techo \"%GX %G\" >expect &&\n+\tgit log -1 --format=\"%GX %G\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success GPG 'detect fudged signature with NUL' '\n \tgit cat-file commit master >raw &&\n \tcat raw >forged2 &&\n-- \n2.0.0.566.gfe3e6b2\n"},{"id":"244358","messageId":"CAPig+cSYETVgS0EPzFbBpEywaWg8k1Yh18rX_8JhJq=Cq5n0WQ@mail.gmail.com","threadId":"36932","inReplyTo":"20140616202611.GA29716@sigill.intra.peff.net","subject":"Re: [PATCH] t7510: check %G* pretty-format output","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-06-16T21:50:14Z","receivedAt":"2014-06-16T21:50:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 16, 2014 at 4:26 PM, Jeff King <peff@peff.net> wrote:\n> On Mon, Jun 16, 2014 at 04:13:11PM -0400, Jeff King wrote:\n>\n>> It doesn't look like we have any tests of \"%G*\" and friends at all. :(\n>\n> Maybe we can add this:\n>\n> -- >8 --\n> Subject: t7510: check %G* pretty-format output\n>\n> We do not check these along with the other pretty-format\n> placeholders in t6006, because we need signed commits to\n> make them interesting. t7510 has such commits, and can\n> easily exercise them in addition to the regular\n> --show-signature code path.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> I explicitly avoided \"%GG\" here, as its exact format is dependent on\n> gpg (and the current date). I don't know that it is worth the\n> complexity to test, as the interesting parts are already parsed from it\n> and exposed in the other placeholders.\n>\n>  t/t7510-signed-commit.sh | 36 ++++++++++++++++++++++++++++++++++++\n>  1 file changed, 36 insertions(+)\n>\n> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\n> index 5ddac1a..2f96937 100755\n> --- a/t/t7510-signed-commit.sh\n> +++ b/t/t7510-signed-commit.sh\n> @@ -76,6 +76,42 @@ test_expect_success GPG 'detect fudged signature' '\n>         ! grep \"Good signature from\" actual1\n>  '\n>\n> +test_expect_success GPG 'show good signature with custom format' '\n> +       cat >expect <<-\\EOF\n\nBroken &&-chain (and in tests below).\n\n> +       G\n> +       13B6F51ECDDE430D\n> +       C O Mitter <committer@example.com>\n> +       EOF\n> +       git log -1 --format=\"%G?%n%GK%n%GS\" master >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n> +test_expect_success GPG 'show bad signature with custom format' '\n> +       cat >expect <<-\\EOF\n> +       B\n> +       13B6F51ECDDE430D\n> +       C O Mitter <committer@example.com>\n> +       EOF\n> +       git log -1 --format=\"%G?%n%GK%n%GS\" $(cat forged1.commit) >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n> +test_expect_success GPG 'show lack of signature with custom format' '\n> +       cat >expect <<-\\EOF\n> +       N\n> +\n> +\n> +       EOF\n> +       git log -1 --format=\"%G?%n%GK%n%GS\" seventh-unsigned >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'unused %G placeholders are passed through' '\n> +       echo \"%GX %G\" >expect &&\n> +       git log -1 --format=\"%GX %G\" >actual &&\n> +       test_cmp expect actual\n> +'\n> +\n>  test_expect_success GPG 'detect fudged signature with NUL' '\n>         git cat-file commit master >raw &&\n>         cat raw >forged2 &&\n> --\n> 2.0.0.566.gfe3e6b2\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"244363","messageId":"20140616233649.GA12350@sigill.intra.peff.net","threadId":"36932","inReplyTo":"CAPig+cSYETVgS0EPzFbBpEywaWg8k1Yh18rX_8JhJq=Cq5n0WQ@mail.gmail.com","subject":"Re: [PATCH] t7510: check %G* pretty-format output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-06-16T23:36:49Z","receivedAt":"2014-06-16T23:36:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 16, 2014 at 05:50:14PM -0400, Eric Sunshine wrote:\n\n> > +test_expect_success GPG 'show good signature with custom format' '\n> > +       cat >expect <<-\\EOF\n> \n> Broken &&-chain (and in tests below).\n\nWhoops, thanks. Re-roll coming in a minute. I'm also reorganizing the\ntwo patches a bit and adding a test for an unvalidated key.\n\n-Peff\n"},{"id":"244364","messageId":"20140616235917.GA19499@sigill.intra.peff.net","threadId":"36932","inReplyTo":"20140616233649.GA12350@sigill.intra.peff.net","subject":"[PATCH 0/5] --format=%G tests and fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-06-16T23:59:17Z","receivedAt":"2014-06-16T23:59:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 16, 2014 at 07:36:49PM -0400, Jeff King wrote:\n\n> On Mon, Jun 16, 2014 at 05:50:14PM -0400, Eric Sunshine wrote:\n> \n> > > +test_expect_success GPG 'show good signature with custom format' '\n> > > +       cat >expect <<-\\EOF\n> > \n> > Broken &&-chain (and in tests below).\n> \n> Whoops, thanks. Re-roll coming in a minute. I'm also reorganizing the\n> two patches a bit and adding a test for an unvalidated key.\n\nOK, here they are. Somehow it multiplied into 5 patches. I must have\ngotten them wet.\n\n  [1/5]: t7510: stop referring to master in later tests\n  [2/5]: t7510: use consistent &&-chains in loop\n  [3/5]: t7510: test a commit signed by an unknown key\n  [4/5]: t7510: check %G* pretty-format output\n  [5/5]: pretty: avoid reading past end-of-string with \"%G\"\n\n-Peff\n"},{"id":"244365","messageId":"20140616235958.GA17110@sigill.intra.peff.net","threadId":"36932","inReplyTo":"20140616235917.GA19499@sigill.intra.peff.net","subject":"[PATCH 1/5] t7510: stop referring to master in later tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-06-16T23:59:59Z","receivedAt":"2014-06-16T23:59:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Our setup creates a sequence of commits, each with its own\ntag. However, we sometimes refer to \"seventh-signed\" as\n\"master\". This works, since it is at the tip of the created\nbranch, but is brittle if new tests need to add more\ncommits. Let's use its tag name to be unambiguous.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t7510-signed-commit.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 5ddac1a..37c3778 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -47,7 +47,7 @@ test_expect_success GPG 'create signed commits' '\n \n test_expect_success GPG 'show signatures' '\n \t(\n-\t\tfor commit in initial second merge fourth-signed fifth-signed sixth-signed master\n+\t\tfor commit in initial second merge fourth-signed fifth-signed sixth-signed seventh-signed\n \t\tdo\n \t\t\tgit show --pretty=short --show-signature $commit >actual &&\n \t\t\tgrep \"Good signature from\" actual || exit 1\n@@ -67,7 +67,7 @@ test_expect_success GPG 'show signatures' '\n '\n \n test_expect_success GPG 'detect fudged signature' '\n-\tgit cat-file commit master >raw &&\n+\tgit cat-file commit seventh-signed >raw &&\n \n \tsed -e \"s/seventh/7th forged/\" raw >forged1 &&\n \tgit hash-object -w -t commit forged1 >forged1.commit &&\n@@ -77,7 +77,7 @@ test_expect_success GPG 'detect fudged signature' '\n '\n \n test_expect_success GPG 'detect fudged signature with NUL' '\n-\tgit cat-file commit master >raw &&\n+\tgit cat-file commit seventh-signed >raw &&\n \tcat raw >forged2 &&\n \techo Qwik | tr \"Q\" \"\\000\" >>forged2 &&\n \tgit hash-object -w -t commit forged2 >forged2.commit &&\n-- \n2.0.0.566.gfe3e6b2\n"},{"id":"244366","messageId":"20140617000343.GB17110@sigill.intra.peff.net","threadId":"36932","inReplyTo":"20140616235917.GA19499@sigill.intra.peff.net","subject":"[PATCH 2/5] t7510: use consistent &&-chains in loop","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-06-17T00:03:43Z","receivedAt":"2014-06-17T00:03:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"From: Michael J Gruber <git@drmicha.warpmail.net>\n\nWe check multiple commits in a loop. Because we want to\nbreak out of the loop if any single iteration fails, we use\na subshell/exit like:\n\n  (\n\tfor i in $stuff\n\tdo\n\t\tdo-something $i || exit 1\n\tdone\n  )\n\nHowever, we are inconsistent in our loop body. Some commands\nget their own \"|| exit 1\", and others try to chain to the\nnext command with \"&&\", like:\n\n  X &&\n  Y || exit 1\n  Z || exit 1\n\nThis is a little hard to read and follow, because X and Y\nare treated differently for no good reason. But much worse,\nthe second loop follows a similar pattern and gets it wrong.\n\"Y\" is expected to fail, so we use \"&& exit 1\", giving us:\n\n  X &&\n  Y && exit 1\n  Z || exit 1\n\nThat gets the test for X wrong (we do not exit unless both X\nfails and Y unexpectedly succeeds, but we would want to exit\nif _either_ is wrong). We can write this clearly and\ncorrectly by consistently using \"&&\", followed by a single\n\"|| exit 1\", and negating Y with \"!\" (as we would in a\nnormal &&-chain). Like:\n\n  X &&\n  ! Y &&\n  Z || exit 1\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI listed Michael as the author here, because this is the patch that I\nexpected to come in his next re-roll, based on our discussion. I'm\nincluding it here because I'm about to touch the same area (and Michael,\nyou'd probably want to rebase on top of this series anyway. Assuming I\nhaven't screwed it up :) ).\n\n t/t7510-signed-commit.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 37c3778..cdffcbd 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -50,18 +50,18 @@ test_expect_success GPG 'show signatures' '\n \t\tfor commit in initial second merge fourth-signed fifth-signed sixth-signed seventh-signed\n \t\tdo\n \t\t\tgit show --pretty=short --show-signature $commit >actual &&\n-\t\t\tgrep \"Good signature from\" actual || exit 1\n-\t\t\t! grep \"BAD signature from\" actual || exit 1\n-\t\t\techo $commit OK\n+\t\t\tgrep \"Good signature from\" actual &&\n+\t\t\t! grep \"BAD signature from\" actual &&\n+\t\t\techo $commit OK || exit 1\n \t\tdone\n \t) &&\n \t(\n \t\tfor commit in merge^2 fourth-unsigned sixth-unsigned seventh-unsigned\n \t\tdo\n \t\t\tgit show --pretty=short --show-signature $commit >actual &&\n-\t\t\tgrep \"Good signature from\" actual && exit 1\n-\t\t\t! grep \"BAD signature from\" actual || exit 1\n-\t\t\techo $commit OK\n+\t\t\t! grep \"Good signature from\" actual &&\n+\t\t\t! grep \"BAD signature from\" actual &&\n+\t\t\techo $commit OK || exit 1\n \t\tdone\n \t)\n '\n-- \n2.0.0.566.gfe3e6b2\n"},{"id":"244367","messageId":"20140617000554.GC17110@sigill.intra.peff.net","threadId":"36932","inReplyTo":"20140616235917.GA19499@sigill.intra.peff.net","subject":"[PATCH 3/5] t7510: test a commit signed by an unknown key","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-06-17T00:05:54Z","receivedAt":"2014-06-17T00:05:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We tested both good and bad signatures, but not ones made\ncorrectly but with a key for which we have no trust.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI'm not happy about grepping more gpg output, but perhaps this \"not\ncertified\" is no worse than the current 'Good signature from\" greps we\nhave?\n\nThe internal code uses --status-fd, which is presumably more robust to\nchanges. I dunno. I'd be inclined to go with this, and if it becomes a\nproblem in a future gpg release, we can deal with it then.\n\n t/t7510-signed-commit.sh | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex cdffcbd..04fc2c5 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -43,6 +43,9 @@ test_expect_success GPG 'create signed commits' '\n \n \ttest_tick && git rebase -f HEAD^^ && git tag sixth-signed HEAD^ &&\n \tgit tag seventh-signed\n+\n+\techo 8 >file && test_tick && git commit -a -m eighth -SB7227189 &&\n+\tgit tag eighth-signed-alt\n '\n \n test_expect_success GPG 'show signatures' '\n@@ -63,6 +66,16 @@ test_expect_success GPG 'show signatures' '\n \t\t\t! grep \"BAD signature from\" actual &&\n \t\t\techo $commit OK || exit 1\n \t\tdone\n+\t) &&\n+\t(\n+\t\tfor commit in eighth-signed-alt\n+\t\tdo\n+\t\t\tgit show --pretty=short --show-signature $commit >actual &&\n+\t\t\tgrep \"Good signature from\" actual &&\n+\t\t\t! grep \"BAD signature from\" actual &&\n+\t\t\tgrep \"not certified\" actual &&\n+\t\t\techo $commit OK || exit 1\n+\t\tdone\n \t)\n '\n \n-- \n2.0.0.566.gfe3e6b2\n"},{"id":"244368","messageId":"20140617000623.GD17110@sigill.intra.peff.net","threadId":"36932","inReplyTo":"20140616235917.GA19499@sigill.intra.peff.net","subject":"[PATCH 4/5] t7510: check %G* pretty-format output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-06-17T00:06:24Z","receivedAt":"2014-06-17T00:06:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We do not check these along with the other pretty-format\nplaceholders in t6006, because we need signed commits to\nmake them interesting. t7510 has such commits, and can\neasily exercise them in addition to the regular\n--show-signature code path.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nSimilar to before, but with the 'U' test added in.\n\n t/t7510-signed-commit.sh | 40 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 40 insertions(+)\n\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 04fc2c5..e97477a 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -107,4 +107,44 @@ test_expect_success GPG 'amending already signed commit' '\n \t! grep \"BAD signature from\" actual\n '\n \n+test_expect_success GPG 'show good signature with custom format' '\n+\tcat >expect <<-\\EOF &&\n+\tG\n+\t13B6F51ECDDE430D\n+\tC O Mitter <committer@example.com>\n+\tEOF\n+\tgit log -1 --format=\"%G?%n%GK%n%GS\" sixth-signed >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show bad signature with custom format' '\n+\tcat >expect <<-\\EOF &&\n+\tB\n+\t13B6F51ECDDE430D\n+\tC O Mitter <committer@example.com>\n+\tEOF\n+\tgit log -1 --format=\"%G?%n%GK%n%GS\" $(cat forged1.commit) >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show unknown signature with custom format' '\n+\tcat >expect <<-\\EOF &&\n+\tU\n+\t61092E85B7227189\n+\tEris Discordia <discord@example.net>\n+\tEOF\n+\tgit log -1 --format=\"%G?%n%GK%n%GS\" eighth-signed-alt >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success GPG 'show lack of signature with custom format' '\n+\tcat >expect <<-\\EOF &&\n+\tN\n+\n+\n+\tEOF\n+\tgit log -1 --format=\"%G?%n%GK%n%GS\" seventh-unsigned >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.0.0.566.gfe3e6b2\n"},{"id":"244370","messageId":"20140617000707.GE17110@sigill.intra.peff.net","threadId":"36932","inReplyTo":"20140616235917.GA19499@sigill.intra.peff.net","subject":"[PATCH 5/5] pretty: avoid reading past end-of-string with \"%G\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-06-17T00:07:07Z","receivedAt":"2014-06-17T00:07:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If the user asks for --format=%G with nothing else, we\ncorrectly realize that \"%G\" is not a valid placeholder (it\nshould be \"%G?\", \"%GK\", etc). But we still tell the\nstrbuf_expand code that we consumed 2 characters, causing it\nto jump over the trailing NUL and output garbage.\n\nThis also fixes the case where \"%GX\" would be consumed (and\nproduce no output). In other cases, we pass unrecognized\nplaceholders through to the final string.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nSame as before, but with the test included along with the fix, rather\nthan in another commit.\n\n pretty.c                 | 2 ++\n t/t7510-signed-commit.sh | 6 ++++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/pretty.c b/pretty.c\nindex e1e2cad..70d8776 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1267,6 +1267,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \t\t\tif (c->signature_check.key)\n \t\t\t\tstrbuf_addstr(sb, c->signature_check.key);\n \t\t\tbreak;\n+\t\tdefault:\n+\t\t\treturn 0;\n \t\t}\n \t\treturn 2;\n \t}\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex e97477a..9810242 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -147,4 +147,10 @@ test_expect_success GPG 'show lack of signature with custom format' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'unused %G placeholders are passed through' '\n+\techo \"%GX %G\" >expect &&\n+\tgit log -1 --format=\"%GX %G\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.0.0.566.gfe3e6b2\n"}]}