Special strings in commit messages

Special strings in commit messages

[Florian Weimer]

I have a commit which looks like this:

$ git cat-file commit 4ca76eb7b47724c2444dfea7890fa8db4edd5762
tree c845be47a0653624b1984d0dc1a0b485b527811d
parent 9eee98638ef06149e17f94afaa357e3a9e296e69
author Florian Weimer <fweimer@redhat.com> 1507481682 +0200
committer Florian Weimer <fweimer@redhat.com> 1507481682 +0200

19: glibc-fedora-nis-rh188246.patch

 From baba5d9461d4e8a581ac26fe4412ad783ffc73e7 Mon Sep 17 00:00:00 2001
From: Jakub Jelinek <jakub@redhat.com>
Date: Mon, 1 May 2006 08:02:53 +0000
Subject: [PATCH] Enable SETENT_BATCH_READ nis/nss option by default

* Mon May  1 2006 Jakub Jelinek <jakub@redhat.com> 2.4.90-4
- SETENT_BATCH_READ /etc/default/nss option for speeding up
   some usages of NIS+ (#188246)


This commit causes git rebase to fail, with this error:

fatal: could not parse .git/rebase-apply/0008

At this point, .git/rebase-apply/0008 contains this:

“
 From baba5d9461d4e8a581ac26fe4412ad783ffc73e7 Mon Sep 17 00:00:00 2001
From: Jakub Jelinek <jakub@redhat.com>
Date: Mon, 1 May 2006 08:02:53 +0000
Subject: [PATCH] Enable SETENT_BATCH_READ nis/nss option by default

* Mon May  1 2006 Jakub Jelinek <jakub@redhat.com> 2.4.90-4
- SETENT_BATCH_READ /etc/default/nss option for speeding up
   some usages of NIS+ (#188246)
---
  nis/nss | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nis/nss b/nis/nss
index 
0ac6774a1ff29f012efaec9c4be1fcc3b83da7e8..d720e719267db5f741b67e7b98e4052e503c4333 
100644
”

Followed by the diff.  The preceding patch, .git/rebase-apply/0007, is:

“
[fweimer@oldenburg glibc-patches]$ cat .git/rebase-apply/0007
 From 4ca76eb7b47724c2444dfea7890fa8db4edd5762 Mon Sep 17 00:00:00 2001
From: Florian Weimer <fweimer@redhat.com>
Date: Sun, 8 Oct 2017 18:54:42 +0200
Subject: 19: glibc-fedora-nis-rh188246.patch


”

Based on strace output, something in git rebase calls git mailsplit, and 
it probably sees the "\nFrom " string and treats it as a start of a new 
mail message, and things go downhill from there.

I will escape "\nFrom " in commit messages (probably as "\n.From " or 
maybe "\n>From ", plus escaping for "\n."/"\n>" to make the encoding 
reversible), but I wonder if there is something else I need to escape 
while I'm at it.

Thanks,
Florian

Re: Special strings in commit messages

[Eric Wong]

Florian Weimer <fweimer@redhat.com> wrote:
> Based on strace output, something in git rebase calls git mailsplit, and it
> probably sees the "\nFrom " string and treats it as a start of a new mail
> message, and things go downhill from there.
> 
> I will escape "\nFrom " in commit messages (probably as "\n.From " or maybe
> "\n>From ", plus escaping for "\n."/"\n>" to make the encoding reversible),
> but I wonder if there is something else I need to escape while I'm at it.

I suppose it's safe to start using mboxrd internally when
there's little danger of mixing different git versions.

Totally untested (but passes "make test"), can you try this?

-----8<------
Subject: [PATCH] rebase: use mboxrd format to avoid split errors

The mboxrd format allows the use of embedded "From " lines in
commit messages without being misinterpreted by mailsplit

Reported-by: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Eric Wong <e@80x24.org>
---
 git-rebase--am.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 6e64d40d6f..14c50782e0 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -53,6 +53,7 @@ else
 
 	git format-patch -k --stdout --full-index --cherry-pick --right-only \
 		--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
+		--pretty=mboxrd \
 		$git_format_patch_opt \
 		"$revisions" ${restrict_revision+^$restrict_revision} \
 		>"$GIT_DIR/rebased-patches"
@@ -83,6 +84,7 @@ else
 	fi
 
 	git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \
+		--patch-format=mboxrd \
 		$allow_rerere_autoupdate \
 		${gpg_sign_opt:+"$gpg_sign_opt"} <"$GIT_DIR/rebased-patches"
 	ret=$?
-- 
EW

Re: Special strings in commit messages

[Florian Weimer]

On 10/08/2017 11:30 PM, Eric Wong wrote:
> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
> index 6e64d40d6f..14c50782e0 100644
> --- a/git-rebase--am.sh
> +++ b/git-rebase--am.sh
> @@ -53,6 +53,7 @@ else
>   
>   	git format-patch -k --stdout --full-index --cherry-pick --right-only \
>   		--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
> +		--pretty=mboxrd \
>   		$git_format_patch_opt \
>   		"$revisions" ${restrict_revision+^$restrict_revision} \
>   		>"$GIT_DIR/rebased-patches"
> @@ -83,6 +84,7 @@ else
>   	fi
>   
>   	git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \
> +		--patch-format=mboxrd \
>   		$allow_rerere_autoupdate \
>   		${gpg_sign_opt:+"$gpg_sign_opt"} <"$GIT_DIR/rebased-patches"
>   	ret=$?

My context is slightly different, but I added the mboxrd options 
manually to both commands, and it fixes my test case.

I'm still wondering if I have to be on the lookout for similar issues 
with different strings.

Thanks,
Florian

[PATCH v2] rebase: use mboxrd format to avoid split errors

[Eric Wong]

Sorry, I forgot about this for a while :x

Florian Weimer <fweimer@redhat.com> wrote:
> On 10/08/2017 11:30 PM, Eric Wong wrote:
> >diff --git a/git-rebase--am.sh b/git-rebase--am.sh
   <snip, identical change below>

> My context is slightly different, but I added the mboxrd options manually to
> both commands, and it fixes my test case.
> 
> I'm still wondering if I have to be on the lookout for similar issues with
> different strings.

I don't think theres other issues with different strings,
"From " lines are the one known ambiguity problem with mbox and
thus the reason mboxrd was created.

Below is an updated patch which adds a test case:

---------8<---------
Subject: [PATCH] rebase: use mboxrd format to avoid split errors

The mboxrd format allows the use of embedded "From " lines in
commit messages without being misinterpreted by mailsplit

Reported-by: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Eric Wong <e@80x24.org>
---
 git-rebase--am.sh |  2 ++
 t/t3400-rebase.sh | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 6e64d40d6f..14c50782e0 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -53,6 +53,7 @@ else
 
 	git format-patch -k --stdout --full-index --cherry-pick --right-only \
 		--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
+		--pretty=mboxrd \
 		$git_format_patch_opt \
 		"$revisions" ${restrict_revision+^$restrict_revision} \
 		>"$GIT_DIR/rebased-patches"
@@ -83,6 +84,7 @@ else
 	fi
 
 	git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" \
+		--patch-format=mboxrd \
 		$allow_rerere_autoupdate \
 		${gpg_sign_opt:+"$gpg_sign_opt"} <"$GIT_DIR/rebased-patches"
 	ret=$?
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index f5fd15e559..8ac58d5ea5 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -255,4 +255,26 @@ test_expect_success 'rebase commit with an ancient timestamp' '
 	grep "author .* 34567 +0600$" actual
 '
 
+test_expect_success 'rebase with "From " line in commit message' '
+	git checkout -b preserve-from master~1 &&
+	cat >From_.msg <<EOF &&
+Somebody embedded an mbox in a commit message
+
+This is from so-and-so:
+
+From a@b Mon Sep 17 00:00:00 2001
+From: John Doe <nobody@example.com>
+Date: Sat, 11 Nov 2017 00:00:00 +0000
+Subject: not this message
+
+something
+EOF
+	>From_ &&
+	git add From_ &&
+	git commit -F From_.msg &&
+	git rebase master &&
+	git log -1 --pretty=format:%B >out &&
+	test_cmp From_.msg out
+'
+
 test_done
-- 
EW