{"thread":{"id":"40524","subject":"[PATCH v2] filter-branch: remove multi-line headers in msg filter","startedAt":"2015-10-09T00:21:13Z","lastAt":"2015-10-12T16:05:29Z","messageCount":5,"participants":["James McCoy","Michael J Gruber","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"271357","messageId":"20151009002113.GA329@freya.jamessan.com","threadId":"40524","inReplyTo":null,"subject":"[PATCH v2] filter-branch: remove multi-line headers in msg filter","fromName":"James McCoy","fromEmail":"vega.james@gmail.com","sentAt":"2015-10-09T00:21:13Z","receivedAt":"2015-10-09T00:21:13Z","isPatch":true,"sender":{"key":"vega.james@gmail.com","avatar":"https://avatars.githubusercontent.com/u/112971?v=4"},"body":"df062010 (filter-branch: avoid passing commit message through sed)\nintroduced a regression when filtering commits with multi-line headers,\nif the header contains a blank line. An example of this is a gpg-signed\ncommit:\n\n $ git cat-file commit signed-commit\n tree 3d4038e029712da9fc59a72afbfcc90418451630\n parent 110eac945dc1713b27bdf49e74e5805db66971f0\n author A U Thor 1112912413 -0700\n committer C O Mitter 1112912413 -0700\n gpgsig -----BEGIN PGP SIGNATURE-----\n Version: GnuPG v1\n\n iEYEABECAAYFAlYXADwACgkQE7b1Hs3eQw23CACgldB/InRyDgQwyiFyMMm3zFpj\n pUsAnA+f3aMUsd9mNroloSmlOgL6jIMO\n =0Hgm\n -----END PGP SIGNATURE-----\n\n Adding gpg\n\nAs a consequence, \"filter-branch --msg-filter cat\" (which should leave the\ncommit message unchanged) spills the signature (after the internal blank\nline) into the original commit message.\n\nThe reason is that although the signature is indented, making the line a\nwhitespace only line, the “read” call is splitting the line based on\nthe shell's IFS, which defaults to . The leading\nspace is consumed and $header_line is empty, causing the “skip header\nlines” loop to exit.\n\nThe rest of the commit object is then re-used as the rewritten commit\nmessage, causing the new message to include the signature of the\noriginal commit.\n\nSet IFS to an empty string for the “read” call, thus disabling the word\nsplitting, which causes $header_line to be set to the non-empty value '\n'. This allows the loop to fully consume the header lines before\nemitting the original, intact commit message.\n\nSigned-off-by: James McCoy \n---\n git-filter-branch.sh | 2 +-\n t/t7003-filter-branch.sh | 14 ++++++++++++++\n 2 files changed, 15 insertions(+), 1 deletion(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 5b3f63d..fff8093 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -347,7 +347,7 @@ while read commit parents; do\n \tfi\n \n \t{\n-\t\twhile read -r header_line && test -n \"$header_line\"\n+\t\twhile IFS='' read -r header_line && test -n \"$header_line\"\n \t\tdo\n \t\t\t# skip header lines...\n \t\t\t:;\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 855afda..377c648 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -2,6 +2,7 @@\n \n test_description='git filter-branch'\n . ./test-lib.sh\n+. \"$TEST_DIRECTORY/lib-gpg.sh\"\n \n test_expect_success 'setup' '\n \ttest_commit A &&\n@@ -292,6 +293,19 @@ test_expect_success 'Tag name filtering strips gpg signature' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success GPG 'Filtering retains message of gpg signed commit' '\n+\tmkdir gpg &&\n+\ttouch gpg/foo &&\n+\tgit add gpg &&\n+\ttest_tick &&\n+\tgit commit -S -m \"Adding gpg\" &&\n+\n+\tgit log -1 --format=\"%s\" > expect &&\n+\tgit filter-branch -f --msg-filter \"cat\" &&\n+\tgit log -1 --format=\"%s\" > actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'Tag name filtering allows slashes in tag names' '\n \tgit tag -m tag-with-slash X/1 &&\n \tgit cat-file tag X/1 | sed -e s,X/1,X/2, > expect &&\n-- \n2.6.1\n\n\n-- \nJames\nGPG Key: 4096R/331BA3DB 2011-12-05 James McCoy \n"},{"id":"271378","messageId":"561767AC.6060503@drmicha.warpmail.net","threadId":"40524","inReplyTo":"20151009002113.GA329@freya.jamessan.com","subject":"Re: [PATCH v2] filter-branch: remove multi-line headers in msg filter","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-10-09T07:07:24Z","receivedAt":"2015-10-09T07:07:24Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"James McCoy venit, vidit, dixit 09.10.2015 02:21:\n> df062010 (filter-branch: avoid passing commit message through sed)\n> introduced a regression when filtering commits with multi-line headers,\n> if the header contains a blank line. An example of this is a gpg-signed\n> commit:\n> \n> $ git cat-file commit signed-commit\n> tree 3d4038e029712da9fc59a72afbfcc90418451630\n> parent 110eac945dc1713b27bdf49e74e5805db66971f0\n> author A U Thor 1112912413 -0700\n> committer C O Mitter 1112912413 -0700\n> gpgsig -----BEGIN PGP SIGNATURE-----\n> Version: GnuPG v1\n> \n> iEYEABECAAYFAlYXADwACgkQE7b1Hs3eQw23CACgldB/InRyDgQwyiFyMMm3zFpj\n> pUsAnA+f3aMUsd9mNroloSmlOgL6jIMO\n> =0Hgm\n> -----END PGP SIGNATURE-----\n> \n> Adding gpg\n> \n> As a consequence, \"filter-branch --msg-filter cat\" (which should leave the\n> commit message unchanged) spills the signature (after the internal blank\n> line) into the original commit message.\n> \n> The reason is that although the signature is indented, making the line a\n> whitespace only line, the “read” call is splitting the line based on\n> the shell's IFS, which defaults to . The leading\n> space is consumed and $header_line is empty, causing the “skip header\n> lines” loop to exit.\n> \n> The rest of the commit object is then re-used as the rewritten commit\n> message, causing the new message to include the signature of the\n> original commit.\n> \n> Set IFS to an empty string for the “read” call, thus disabling the word\n> splitting, which causes $header_line to be set to the non-empty value '\n> '. This allows the loop to fully consume the header lines before\n> emitting the original, intact commit message.\n> \n> Signed-off-by: James McCoy \n> ---\n\nThanks for hanging in :)\n\nReviewed-by: Michael J Gruber \n"},{"id":"271384","messageId":"xmqqpp0oormv.fsf@gitster.mtv.corp.google.com","threadId":"40524","inReplyTo":"561767AC.6060503@drmicha.warpmail.net","subject":"Re: [PATCH v2] filter-branch: remove multi-line headers in msg filter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-09T17:53:28Z","receivedAt":"2015-10-09T17:53:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n>> Set IFS to an empty string for the “read” call, thus disabling the word\n>> splitting, which causes $header_line to be set to the non-empty value '\n>> '. This allows the loop to fully consume the header lines before\n>> emitting the original, intact commit message.\n>> \n>> Signed-off-by: James McCoy \n>> ---\n>\n> Thanks for hanging in :)\n>\n> Reviewed-by: Michael J Gruber \n\nAs long as you are fine with giving authorship to James, I am fine\nwith that. I'll amend what is queued with your reviewed-by above\nand will merge to 'next'.\n\nThanks.\n"},{"id":"271437","messageId":"561B60B1.10003@drmicha.warpmail.net","threadId":"40524","inReplyTo":"xmqqpp0oormv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] filter-branch: remove multi-line headers in msg filter","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-10-12T07:26:41Z","receivedAt":"2015-10-12T07:26:41Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 09.10.2015 19:53:\n> Michael J Gruber writes:\n> \n>>> Set IFS to an empty string for the “read” call, thus disabling the word\n>>> splitting, which causes $header_line to be set to the non-empty value '\n>>> '. This allows the loop to fully consume the header lines before\n>>> emitting the original, intact commit message.\n>>>\n>>> Signed-off-by: James McCoy \n>>> ---\n>>\n>> Thanks for hanging in :)\n>>\n>> Reviewed-by: Michael J Gruber \n> \n> As long as you are fine with giving authorship to James, I am fine\n> with that. I'll amend what is queued with your reviewed-by above\n> and will merge to 'next'.\n\nYep, I'm fine with \"mini-mentoring\", and just to be safe, the 7 new\ncharacters in git-filter-branch.sh are (also)\n\nSigned-off-by: Michael J Gruber \n\nin case that is needed or preferred.\n\nMichael\n"},{"id":"271453","messageId":"xmqqtwpwm5rq.fsf@gitster.mtv.corp.google.com","threadId":"40524","inReplyTo":"561B60B1.10003@drmicha.warpmail.net","subject":"Re: [PATCH v2] filter-branch: remove multi-line headers in msg filter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-12T16:05:29Z","receivedAt":"2015-10-12T16:05:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Yep, I'm fine with \"mini-mentoring\", and just to be safe, the 7 new\n> characters in git-filter-branch.sh are (also)\n>\n> Signed-off-by: Michael J Gruber \n>\n> in case that is needed or preferred.\n\nI was wondering if we want to do that or I can just add comments\n\"based on MJG's suggestion\" or somesuch. Most likely I'd do both\n;-)\n\nThanks.\n"}]}