threads / patch / 40524

v2filter-branch: remove multi-line headers in msg filter

Subject: [PATCH v2] filter-branch: remove multi-line headers in msg filter

## tl;dr

5 messages between Oct 9, 2015 and Oct 12, 2015. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

James McCoy· Oct 9, 2015, 00:21 UTC · lore

df062010 (filter-branch: avoid passing commit message through sed) introduced a regression when filtering commits with multi-line headers, if the header contains a blank line. An example of this is a gpg-signed commit:

  $ git cat-file commit signed-commit
  tree 3d4038e029712da9fc59a72afbfcc90418451630
  parent 110eac945dc1713b27bdf49e74e5805db66971f0
  author A U Thor <author@example.com> 1112912413 -0700
  committer C O Mitter <committer@example.com> 1112912413 -0700
  gpgsig -----BEGIN PGP SIGNATURE-----
   Version: GnuPG v1
   iEYEABECAAYFAlYXADwACgkQE7b1Hs3eQw23CACgldB/InRyDgQwyiFyMMm3zFpj
   pUsAnA+f3aMUsd9mNroloSmlOgL6jIMO
   =0Hgm
   -----END PGP SIGNATURE-----
  Adding gpg

As a consequence, "filter-branch --msg-filter cat" (which should leave the commit message unchanged) spills the signature (after the internal blank line) into the original commit message.

The reason is that although the signature is indented, making the line a whitespace only line, the “read” call is splitting the line based on the shell's IFS, which defaults to <space><tab><newline>. The leading space is consumed and $header_line is empty, causing the “skip header lines” loop to exit.

The rest of the commit object is then re-used as the rewritten commit message, causing the new message to include the signature of the original commit.

Set IFS to an empty string for the “read” call, thus disabling the word splitting, which causes $header_line to be set to the non-empty value ' '. This allows the loop to fully consume the header lines before emitting the original, intact commit message.

Signed-off-by: James McCoy <vega.james@gmail.com>
---
 git-filter-branch.sh     |  2 +-
 t/t7003-filter-branch.sh | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)
Show changes to 2 files +15 −1

git-filter-branch.sh, t/t7003-filter-branch.sh

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 5b3f63d..fff8093 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -347,7 +347,7 @@ while read commit parents; do
 	fi
 
 	{
-		while read -r header_line && test -n "$header_line"
+		while IFS='' read -r header_line && test -n "$header_line"
 		do
 			# skip header lines...
 			:;
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 855afda..377c648 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -2,6 +2,7 @@
 
 test_description='git filter-branch'
 . ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
 
 test_expect_success 'setup' '
 	test_commit A &&
@@ -292,6 +293,19 @@ test_expect_success 'Tag name filtering strips gpg signature' '
 	test_cmp expect actual
 '
 
+test_expect_success GPG 'Filtering retains message of gpg signed commit' '
+	mkdir gpg &&
+	touch gpg/foo &&
+	git add gpg &&
+	test_tick &&
+	git commit -S -m "Adding gpg" &&
+
+	git log -1 --format="%s" > expect &&
+	git filter-branch -f --msg-filter "cat" &&
+	git log -1 --format="%s" > actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'Tag name filtering allows slashes in tag names' '
 	git tag -m tag-with-slash X/1 &&
 	git cat-file tag X/1 | sed -e s,X/1,X/2, > expect &&
-- 
2.6.1


-- 
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy <jamessan@debian.org>
Michael J Gruber· Oct 9, 2015, 07:07 UTC · re: James McCoy · lore

Re: [PATCH v2] filter-branch: remove multi-line headers in msg filter

James McCoy venit, vidit, dixit 09.10.2015 02:21:
Show 41 quoted lines
> df062010 (filter-branch: avoid passing commit message through sed)
> introduced a regression when filtering commits with multi-line headers,
> if the header contains a blank line.  An example of this is a gpg-signed
> commit:
> 
>   $ git cat-file commit signed-commit
>   tree 3d4038e029712da9fc59a72afbfcc90418451630
>   parent 110eac945dc1713b27bdf49e74e5805db66971f0
>   author A U Thor <author@example.com> 1112912413 -0700
>   committer C O Mitter <committer@example.com> 1112912413 -0700
>   gpgsig -----BEGIN PGP SIGNATURE-----
>    Version: GnuPG v1
> 
>    iEYEABECAAYFAlYXADwACgkQE7b1Hs3eQw23CACgldB/InRyDgQwyiFyMMm3zFpj
>    pUsAnA+f3aMUsd9mNroloSmlOgL6jIMO
>    =0Hgm
>    -----END PGP SIGNATURE-----
> 
>   Adding gpg
> 
> As a consequence, "filter-branch --msg-filter cat" (which should leave the
> commit message unchanged) spills the signature (after the internal blank
> line) into the original commit message.
> 
> The reason is that although the signature is indented, making the line a
> whitespace only line, the “read” call is splitting the line based on
> the shell's IFS, which defaults to <space><tab><newline>.  The leading
> space is consumed and $header_line is empty, causing the “skip header
> lines” loop to exit.
> 
> The rest of the commit object is then re-used as the rewritten commit
> message, causing the new message to include the signature of the
> original commit.
> 
> Set IFS to an empty string for the “read” call, thus disabling the word
> splitting, which causes $header_line to be set to the non-empty value '
> '.  This allows the loop to fully consume the header lines before
> emitting the original, intact commit message.
> 
> Signed-off-by: James McCoy <vega.james@gmail.com>
> ---
Thanks for hanging in :)
Reviewed-by: Michael J Gruber <git@drmicha.warpmail.net>
Junio C Hamano· Oct 9, 2015, 17:53 UTC · re: Michael J Gruber · lore

Re: [PATCH v2] filter-branch: remove multi-line headers in msg filter

Michael J Gruber <git@drmicha.warpmail.net> writes:
Show 11 quoted lines
>> Set IFS to an empty string for the “read” call, thus disabling the word
>> splitting, which causes $header_line to be set to the non-empty value '
>> '.  This allows the loop to fully consume the header lines before
>> emitting the original, intact commit message.
>> 
>> Signed-off-by: James McCoy <vega.james@gmail.com>
>> ---
>
> Thanks for hanging in :)
>
> Reviewed-by: Michael J Gruber <git@drmicha.warpmail.net>

As long as you are fine with giving authorship to James, I am fine with that. I'll amend what is queued with your reviewed-by above and will merge to 'next'.

Thanks.
Michael J Gruber· Oct 12, 2015, 07:26 UTC · re: Junio C Hamano · lore

Re: [PATCH v2] filter-branch: remove multi-line headers in msg filter

Junio C Hamano venit, vidit, dixit 09.10.2015 19:53:
Show 17 quoted lines
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>>> Set IFS to an empty string for the “read” call, thus disabling the word
>>> splitting, which causes $header_line to be set to the non-empty value '
>>> '.  This allows the loop to fully consume the header lines before
>>> emitting the original, intact commit message.
>>>
>>> Signed-off-by: James McCoy <vega.james@gmail.com>
>>> ---
>>
>> Thanks for hanging in :)
>>
>> Reviewed-by: Michael J Gruber <git@drmicha.warpmail.net>
> 
> As long as you are fine with giving authorship to James, I am fine
> with that.  I'll amend what is queued with your reviewed-by above
> and will merge to 'next'.

Yep, I'm fine with "mini-mentoring", and just to be safe, the 7 new characters in git-filter-branch.sh are (also)

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
in case that is needed or preferred.
Michael
Junio C Hamano· Oct 12, 2015, 16:05 UTC · re: Michael J Gruber · lore

Re: [PATCH v2] filter-branch: remove multi-line headers in msg filter

Michael J Gruber <git@drmicha.warpmail.net> writes:
Show 6 quoted lines
> Yep, I'm fine with "mini-mentoring", and just to be safe, the 7 new
> characters in git-filter-branch.sh are (also)
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>
> in case that is needed or preferred.

I was wondering if we want to do that or I can just add comments "based on MJG's suggestion" or somesuch. Most likely I'd do both ;-)

Thanks.

← back to recent threads