Commit 8479284323dc11b5f08a6d8896bd806722b34634

Stefan Sperling 2019-08-09T12:31:18

attempt to reject GOT_AUTHOR values without an email address

diff --git a/got/got.1 b/got/got.1
index a6c2e61..967828d 100644
--- a/got/got.1
+++ b/got/got.1
@@ -1081,12 +1081,13 @@ and
 .Cm got import ,
 for example:
 .An Flan Hacker Aq Mt flan_hacker@openbsd.org .
+Because
 .Xr git 1
-may fail to parse commits created with
+may fail to parse commits without an email address in author data,
 .Nm
-if the
+attempts to reject
 .Ev GOT_AUTHOR
-environment variable does not contain an email address.
+environment variables with a missing email address.
 .It Ev VISUAL, Ev EDITOR
 The editor spawned by
 .Cm got commit .
diff --git a/got/got.c b/got/got.c
index 7042801..c05635e 100644
--- a/got/got.c
+++ b/got/got.c
@@ -483,12 +483,47 @@ import_progress(void *arg, const char *path)
 }
 
 static const struct got_error *
+get_author(const char **author)
+{
+	const char *got_author;
+
+	*author = NULL;
+
+	got_author = getenv("GOT_AUTHOR");
+	if (got_author == NULL) {
+		/* TODO: Look up user in password database? */
+		return got_error(GOT_ERR_COMMIT_NO_AUTHOR);
+	}
+
+	*author = got_author;
+
+	/*
+	 * Really dumb email address check; we're only doing this to
+	 * avoid git's object parser breaking on commits we create.
+	 */
+	while (*got_author && *got_author != '<')
+		got_author++;
+	if (*got_author != '<')
+		return got_error(GOT_ERR_COMMIT_NO_EMAIL);
+	while (*got_author && *got_author != '@')
+		got_author++;
+	if (*got_author != '@')
+		return got_error(GOT_ERR_COMMIT_NO_EMAIL);
+	while (*got_author && *got_author != '>')
+		got_author++;
+	if (*got_author != '>')
+		return got_error(GOT_ERR_COMMIT_NO_EMAIL);
+
+	return NULL;
+}
+
+static const struct got_error *
 cmd_import(int argc, char *argv[])
 {
 	const struct got_error *error = NULL;
 	char *path_dir = NULL, *repo_path = NULL, *logmsg = NULL;
 	char *editor = NULL;
-	const char *got_author = getenv("GOT_AUTHOR");
+	const char *author;
 	const char *branch_name = "master";
 	char *refname = NULL, *id_str = NULL;
 	struct got_repository *repo = NULL;
@@ -544,11 +579,9 @@ cmd_import(int argc, char *argv[])
 	if (argc != 1)
 		usage_import();
 
-	if (got_author == NULL) {
-		/* TODO: Look current user up in password database */
-		error = got_error(GOT_ERR_COMMIT_NO_AUTHOR);
-		goto done;
-	}
+	error = get_author(&author);
+	if (error)
+		return error;
 
 	if (repo_path == NULL) {
 		repo_path = getcwd(NULL, 0);
@@ -603,7 +636,7 @@ cmd_import(int argc, char *argv[])
 		goto done;
 
 	error = got_repo_import(&new_commit_id, path_dir, logmsg,
-	    got_author, &ignores, repo, import_progress, NULL);
+	    author, &ignores, repo, import_progress, NULL);
 	if (error)
 		goto done;
 
@@ -3447,7 +3480,7 @@ cmd_commit(int argc, char *argv[])
 	char *cwd = NULL, *id_str = NULL;
 	struct got_object_id *id = NULL;
 	const char *logmsg = NULL;
-	const char *got_author = getenv("GOT_AUTHOR");
+	const char *author;
 	struct collect_commit_logmsg_arg cl_arg;
 	char *editor = NULL;
 	int ch, rebase_in_progress, histedit_in_progress;
@@ -3475,11 +3508,9 @@ cmd_commit(int argc, char *argv[])
 	    "unveil", NULL) == -1)
 		err(1, "pledge");
 #endif
-	if (got_author == NULL) {
-		/* TODO: Look current user up in password database */
-		error = got_error(GOT_ERR_COMMIT_NO_AUTHOR);
-		goto done;
-	}
+	error = get_author(&author);
+	if (error)
+		return error;
 
 	cwd = getcwd(NULL, 0);
 	if (cwd == NULL) {
@@ -3535,7 +3566,7 @@ cmd_commit(int argc, char *argv[])
 		cl_arg.branch_name += 11;
 	}
 	cl_arg.repo_path = got_repo_get_path(repo);
-	error = got_worktree_commit(&id, worktree, &paths, got_author, NULL,
+	error = got_worktree_commit(&id, worktree, &paths, author, NULL,
 	    collect_commit_logmsg, &cl_arg, print_status, NULL, repo);
 	if (error) {
 		if (cl_arg.logmsg_path)
diff --git a/include/got_error.h b/include/got_error.h
index cd6c014..884a46a 100644
--- a/include/got_error.h
+++ b/include/got_error.h
@@ -121,6 +121,7 @@
 #define GOT_ERR_FILE_NOT_STAGED 105
 #define GOT_ERR_STAGED_PATHS	106
 #define GOT_ERR_PATCH_CHOICE	107
+#define GOT_ERR_COMMIT_NO_EMAIL	108
 
 static const struct got_error {
 	int code;
@@ -246,6 +247,9 @@ static const struct got_error {
 	{ GOT_ERR_STAGED_PATHS, "work tree contains files with staged "
 	    "changes; these changes must be committed or unstaged first" },
 	{ GOT_ERR_PATCH_CHOICE, "invalid patch choice" },
+	{ GOT_ERR_COMMIT_NO_EMAIL,"GOT_AUTHOR environment variable contains "
+	    "no email address; an email address is required for compatibility "
+	    "with Git" },
 };
 
 /*
diff --git a/regress/cmdline/commit.sh b/regress/cmdline/commit.sh
index 385f2ae..337efdc 100755
--- a/regress/cmdline/commit.sh
+++ b/regress/cmdline/commit.sh
@@ -541,6 +541,44 @@ function test_commit_outside_refs_heads {
 	test_done "$testroot" "$ret"
 }
 
+function test_commit_no_email {
+	local testroot=`test_init commit_no_email`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "modified alpha" > $testroot/wt/alpha
+	(cd $testroot/wt && env GOT_AUTHOR=":flan_hacker:" \
+		got commit -m 'test no email' > $testroot/stdout \
+		2> $testroot/stderr)
+
+	echo -n "got: GOT_AUTHOR environment variable contains no email " \
+		> $testroot/stderr.expected
+	echo -n "address; an email address is required for compatibility "\
+		>> $testroot/stderr.expected
+	echo "with Git" >> $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret="$?"
+	if [ "$ret" != "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" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" "$ret"
+}
+
+
 
 run_test test_commit_basic
 run_test test_commit_new_subdir
@@ -555,3 +593,4 @@ run_test test_commit_path_prefix
 run_test test_commit_dir_path
 run_test test_commit_selected_paths
 run_test test_commit_outside_refs_heads
+run_test test_commit_no_email