commit cf208ddd0ee1b6c232ef38bc307e689bc2b4653b
from: Omar Polo <op@omarpolo.com>
date: Tue Jul 19 10:02:45 2022 UTC

fix email address parsing

we were both too strict and too lose.  To avoid breaking got object
parser (and to some extent ours too) we need to ensure that there aren't
any line feeds, extra < or > and no trailing gibberish.  The '@' is not
actually required in the email.

various tweaks and ok stsp

commit - 10aab77fd5164e672b109e74278bc080a7546fb8
commit + cf208ddd0ee1b6c232ef38bc307e689bc2b4653b
blob - b7c305dd8fba01e7b8b4d08eae548d3f7c53246c
blob + fc3f426e58ce1bbe4245627188786b1b9227e1c0
--- got/got.c
+++ got/got.c
@@ -548,24 +548,28 @@ import_progress(void *arg, const char *path)
 	return NULL;
 }
 
-static int
+static const struct got_error *
 valid_author(const char *author)
 {
+	const char *email = author;
+
 	/*
-	 * Really dumb email address check; we're only doing this to
-	 * avoid git's object parser breaking on commits we create.
+	 * Git' expects the author (or committer) to be in the form
+	 * "name <email>", which are mostly free form (see the
+	 * "committer" description in git-fast-import(1)).  We're only
+	 * doing this to avoid git's object parser breaking on commits
+	 * we create.
 	 */
-	while (*author && *author != '<')
-		author++;
-	if (*author != '<')
-		return 0;
-	while (*author && *author != '@')
+
+	while (*author && *author != '\n' && *author != '<' && *author != '>')
 		author++;
-	if (*author != '@')
-		return 0;
-	while (*author && *author != '>')
+	if (*author++ != '<')
+		return got_error_fmt(GOT_ERR_COMMIT_NO_EMAIL, "%s", email);
+	while (*author && *author != '\n' && *author != '<' && *author != '>')
 		author++;
-	return *author == '>';
+	if (strcmp(author, ">") != 0)
+		return got_error_fmt(GOT_ERR_COMMIT_NO_EMAIL, "%s", email);
+	return NULL;
 }
 
 static const struct got_error *
@@ -625,8 +629,8 @@ get_author(char **author, struct got_repository *repo,
 	if (*author == NULL)
 		return got_error_from_errno("strdup");
 
-	if (!valid_author(*author)) {
-		err = got_error_fmt(GOT_ERR_COMMIT_NO_EMAIL, "%s", *author);
+	err = valid_author(*author);
+	if (err) {
 		free(*author);
 		*author = NULL;
 	}
blob - 4fe2c68a57b89e39bec69f8c2dbcf24f0f265384
blob + 74b1300abdc8bad6089c22251a9efd9642490d49
--- regress/cmdline/commit.sh
+++ regress/cmdline/commit.sh
@@ -640,6 +640,10 @@ test_commit_outside_refs_heads() {
 
 test_commit_no_email() {
 	local testroot=`test_init commit_no_email`
+	local errmsg=""
+
+	errmsg="commit author's email address is required for"
+	errmsg="$errmsg compatibility with Git"
 
 	got checkout $testroot/repo $testroot/wt > /dev/null
 	ret=$?
@@ -653,10 +657,7 @@ test_commit_no_email() {
 		got commit -m 'test no email' > $testroot/stdout \
 		2> $testroot/stderr)
 
-	echo -n "got: :flan_hacker:: commit author's email address " \
-		> $testroot/stderr.expected
-	echo "is required for compatibility with Git" \
-		>> $testroot/stderr.expected
+	printf "got: :flan_hacker:: %s\n" "$errmsg" > $testroot/stderr.expected
 	cmp -s $testroot/stderr.expected $testroot/stderr
 	ret=$?
 	if [ $ret -ne 0 ]; then
@@ -670,8 +671,56 @@ test_commit_no_email() {
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" $ret
+		return 1
 	fi
-	test_done "$testroot" "$ret"
+
+	# try again with a newline inside the email
+	(cd $testroot/wt \
+		&& FS=' ' env GOT_AUTHOR="$(printf "Flan <hack\ner>")" \
+		got commit -m 'test invalid email' > $testroot/stdout \
+		2> $testroot/stderr)
+
+	printf "got: Flan <hack\ner>: %s\n" "$errmsg" \
+		> $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	echo -n > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	# try again with a < inside the email
+	(cd $testroot/wt && env GOT_AUTHOR="$(printf "Flan <ha<ker>")" \
+		got commit -m 'test invalid email' > $testroot/stdout \
+		2> $testroot/stderr)
+
+	printf "got: Flan <ha<ker>: %s\n" "$errmsg" > $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	echo -n > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" $ret
 }
 
 test_commit_tree_entry_sorting() {