git rebase loses author name/email if given bad email address

git rebase loses author name/email if given bad email address

[Amos Waterland]

If GIT_AUTHOR_EMAIL is of a certain form, `git rebase master' will blow
away the author name and email when fast-forward merging commits.  I
have not tracked it down, but here is a testcase that demonstrates the
behavior.

Signed-off-by: Amos Waterland <apw@us.ibm.com>
Acked-by: Michal Ostrowski <mostrows@watson.ibm.com>

---

 t/t3400-rebase.sh |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)
 create mode 100755 t/t3400-rebase.sh

6a31c31a8e6bf4328fab39f39926af6bcc794bf2
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
new file mode 100755
index 0000000..b9d3131
--- /dev/null
+++ b/t/t3400-rebase.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Amos Waterland
+#
+
+test_description='git rebase should not destroy author information
+
+This test runs git rebase and checks that the author information is not lost.
+'
+. ./test-lib.sh
+
+export GIT_AUTHOR_EMAIL=bogus_email_address
+
+test_expect_success \
+    'prepare repository with topic branch, then rebase against master' \
+    'echo First > A &&
+     git-update-index --add A &&
+     git-commit -m "Add A." &&
+     git checkout -b my-topic-branch &&
+     echo Second > B &&
+     git-update-index --add B &&
+     git-commit -m "Add B." &&
+     git checkout -f master &&
+     echo Third >> A &&
+     git-update-index A &&
+     git-commit -m "Modify A." &&
+     git checkout -f my-topic-branch &&
+     git rebase master'
+
+test_expect_failure \
+    'the rebase operation should not have destroyed author information' \
+    'git log | grep "Author:" | grep "<>"'
+
+test_done
-- 
0.99.9.GIT

Re: git rebase loses author name/email if given bad email address

[Junio C Hamano]

Amos Waterland <apw@us.ibm.com> writes:

> If GIT_AUTHOR_EMAIL is of a certain form, `git rebase master' will blow
> away the author name and email when fast-forward merging commits.

True.

> +export GIT_AUTHOR_EMAIL=bogus_email_address

Do not do that, then ;-).

The rebasing involves extracting the patch as an e-mail form and
feeding it to the normal e-mail patch acceptance mechanism, and
mailinfo.c::handle_from() rejects "A U Thor <bogus_address>"
hence you lose the authorship information.

Having said that, maybe e-mail acceptance machinery should
reject such patch.

-- >8 --
Subject: [PATCH] mailinfo and git-am: allow "John Doe <johndoe>"

An isolated developer could have a local-only e-mail, which will
be stripped out by mailinfo because it lacks '@'.  Define a
fallback parser to accomodate that.

At the same time, reject authorless patch in git-am.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 git-am.sh  |    7 +++++++
 mailinfo.c |   32 +++++++++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 1 deletions(-)

e0e3ba208d235ab5623a86204fbd20b449520764
diff --git a/git-am.sh b/git-am.sh
index 343bee9..1a114bc 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -249,6 +249,13 @@ do
 	GIT_AUTHOR_NAME="$(sed -n '/^Author/ s/Author: //p' "$dotest/info")"
 	GIT_AUTHOR_EMAIL="$(sed -n '/^Email/ s/Email: //p' "$dotest/info")"
 	GIT_AUTHOR_DATE="$(sed -n '/^Date/ s/Date: //p' "$dotest/info")"
+
+	if test -z "$GIT_AUTHOR_EMAIL"
+	then
+		echo "Patch does not have a valid e-mail address."
+		stop_here $this
+	fi
+
 	export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE
 
 	SUBJECT="$(sed -n '/^Subject/ s/Subject: //p' "$dotest/info")"
diff --git a/mailinfo.c b/mailinfo.c
index d4b4163..9f95f37 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -40,13 +40,43 @@ static char *sanity_check(char *name, ch
 	return name;
 }
 
+static int bogus_from(char *line)
+{
+	/* John Doe <johndoe> */
+	char *bra, *ket, *dst, *cp;
+
+	/* This is fallback, so do not bother if we already have an
+	 * e-mail address.
+	 */ 
+	if (*email)
+		return 0;
+
+	bra = strchr(line, '<');
+	if (!bra)
+		return 0;
+	ket = strchr(bra, '>');
+	if (!ket)
+		return 0;
+
+	for (dst = email, cp = bra+1; cp < ket; )
+		*dst++ = *cp++;
+	*dst = 0;
+	for (cp = line; isspace(*cp); cp++)
+		;
+	for (bra--; isspace(*bra); bra--)
+		*bra = 0;
+	cp = sanity_check(cp, email);
+	strcpy(name, cp);
+	return 1;
+}
+
 static int handle_from(char *line)
 {
 	char *at = strchr(line, '@');
 	char *dst;
 
 	if (!at)
-		return 0;
+		return bogus_from(line);
 
 	/*
 	 * If we already have one email, don't take any confusing lines
-- 
0.99.9m