From afb6c30b27069db666a0004f4b6b05c067170088 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Thu, 23 Feb 2017 03:12:30 -0500
Subject: [PATCH 1/4] ident: mark error messages for translation

We already translate the big "please tell me who you are"
hint, but missed the individual error messages that go with
it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 ident.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ident.c b/ident.c
index ac4ae02b48..dde82983ad 100644
--- a/ident.c
+++ b/ident.c
@@ -357,13 +357,13 @@ const char *fmt_ident(const char *name, const char *email,
 			if (strict && ident_use_config_only
 			    && !(ident_config_given & IDENT_NAME_GIVEN)) {
 				fputs(_(env_hint), stderr);
-				die("no name was given and auto-detection is disabled");
+				die(_("no name was given and auto-detection is disabled"));
 			}
 			name = ident_default_name();
 			using_default = 1;
 			if (strict && default_name_is_bogus) {
 				fputs(_(env_hint), stderr);
-				die("unable to auto-detect name (got '%s')", name);
+				die(_("unable to auto-detect name (got '%s')"), name);
 			}
 		}
 		if (!*name) {
@@ -371,7 +371,7 @@ const char *fmt_ident(const char *name, const char *email,
 			if (strict) {
 				if (using_default)
 					fputs(_(env_hint), stderr);
-				die("empty ident name (for <%s>) not allowed", email);
+				die(_("empty ident name (for <%s>) not allowed"), email);
 			}
 			pw = xgetpwuid_self(NULL);
 			name = pw->pw_name;
@@ -382,12 +382,12 @@ const char *fmt_ident(const char *name, const char *email,
 		if (strict && ident_use_config_only
 		    && !(ident_config_given & IDENT_MAIL_GIVEN)) {
 			fputs(_(env_hint), stderr);
-			die("no email was given and auto-detection is disabled");
+			die(_("no email was given and auto-detection is disabled"));
 		}
 		email = ident_default_email();
 		if (strict && default_email_is_bogus) {
 			fputs(_(env_hint), stderr);
-			die("unable to auto-detect email address (got '%s')", email);
+			die(_("unable to auto-detect email address (got '%s')"), email);
 		}
 	}
 
@@ -403,7 +403,7 @@ const char *fmt_ident(const char *name, const char *email,
 		strbuf_addch(&ident, ' ');
 		if (date_str && date_str[0]) {
 			if (parse_date(date_str, &ident) < 0)
-				die("invalid date format: %s", date_str);
+				die(_("invalid date format: %s"), date_str);
 		}
 		else
 			strbuf_addstr(&ident, ident_default_date());

From 862e80a413e45d34834ecd573e5c6b39e38ba850 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Thu, 23 Feb 2017 03:13:53 -0500
Subject: [PATCH 2/4] ident: handle NULL email when complaining of empty name

If we see an empty name, we complain about and mention the
matching email in the error message (to give it some
context). However, the "email" pointer may be NULL here if
we were planning to fill it in later from ident_default_email().

This was broken by 59f929596 (fmt_ident: refactor strictness
checks, 2016-02-04). Prior to that commit, we would look up
the default name and email before doing any other actions.
So one solution would be to go back to that.

However, we can't just do so blindly. The logic for handling
the "!email" condition has grown since then. In particular,
looking up the default email can die if getpwuid() fails,
but there are other errors that should take precedence.
Commit 734c7789a (ident: check for useConfigOnly before
auto-detection of name/email, 2016-03-30) reordered the
checks so that we prefer the error message for
useConfigOnly.

Instead, we can observe that while the name-handling depends
on "email" being set, the reverse is not true. So we can
simply set up the email variable first.

This does mean that if both are bogus, we'll complain about
the email before the name. But between the two, there is no
reason to prefer one over the other.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 ident.c                       | 26 +++++++++++++-------------
 t/t7518-ident-corner-cases.sh | 20 ++++++++++++++++++++
 2 files changed, 33 insertions(+), 13 deletions(-)
 create mode 100755 t/t7518-ident-corner-cases.sh

diff --git a/ident.c b/ident.c
index dde82983ad..ea6034581c 100644
--- a/ident.c
+++ b/ident.c
@@ -351,6 +351,19 @@ const char *fmt_ident(const char *name, const char *email,
 	int want_date = !(flag & IDENT_NO_DATE);
 	int want_name = !(flag & IDENT_NO_NAME);
 
+	if (!email) {
+		if (strict && ident_use_config_only
+		    && !(ident_config_given & IDENT_MAIL_GIVEN)) {
+			fputs(_(env_hint), stderr);
+			die(_("no email was given and auto-detection is disabled"));
+		}
+		email = ident_default_email();
+		if (strict && default_email_is_bogus) {
+			fputs(_(env_hint), stderr);
+			die(_("unable to auto-detect email address (got '%s')"), email);
+		}
+	}
+
 	if (want_name) {
 		int using_default = 0;
 		if (!name) {
@@ -378,19 +391,6 @@ const char *fmt_ident(const char *name, const char *email,
 		}
 	}
 
-	if (!email) {
-		if (strict && ident_use_config_only
-		    && !(ident_config_given & IDENT_MAIL_GIVEN)) {
-			fputs(_(env_hint), stderr);
-			die(_("no email was given and auto-detection is disabled"));
-		}
-		email = ident_default_email();
-		if (strict && default_email_is_bogus) {
-			fputs(_(env_hint), stderr);
-			die(_("unable to auto-detect email address (got '%s')"), email);
-		}
-	}
-
 	strbuf_reset(&ident);
 	if (want_name) {
 		strbuf_addstr_without_crud(&ident, name);
diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh
new file mode 100755
index 0000000000..6c057afc11
--- /dev/null
+++ b/t/t7518-ident-corner-cases.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+test_description='corner cases in ident strings'
+. ./test-lib.sh
+
+# confirm that we do not segfault _and_ that we do not say "(null)", as
+# glibc systems will quietly handle our NULL pointer
+#
+# Note also that we can't use "env" here because we need to unset a variable,
+# and "-u" is not portable.
+test_expect_success 'empty name and missing email' '
+	(
+		sane_unset GIT_AUTHOR_EMAIL &&
+		GIT_AUTHOR_NAME= &&
+		test_must_fail git commit --allow-empty -m foo 2>err &&
+		test_i18ngrep ! null err
+	)
+'
+
+test_done

From 13b9a24e58f736b70e48846cf7e5b7cfa66c3fec Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Thu, 23 Feb 2017 03:15:55 -0500
Subject: [PATCH 3/4] ident: reject all-crud ident name

An ident name consisting of only "crud" characters (like
whitespace or punctuation) is effectively the same as an
empty one, because our strbuf_addstr_without_crud() will
remove those characters.

We reject an empty name when formatting a strict ident, but
don't notice an all-crud one because our check happens
before the crud-removal step.

We could skip past the crud before checking for an empty
name, but let's make it a separate code path, for two
reasons. One is that we can give a more specific error
message. And two is that unlike a blank name, we probably
don't want to kick in the fallback-to-username behavior.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 ident.c                       | 11 +++++++++++
 t/t7518-ident-corner-cases.sh |  5 +++++
 2 files changed, 16 insertions(+)

diff --git a/ident.c b/ident.c
index ea6034581c..ead09ff7f6 100644
--- a/ident.c
+++ b/ident.c
@@ -203,6 +203,15 @@ static int crud(unsigned char c)
 		c == '\'';
 }
 
+static int has_non_crud(const char *str)
+{
+	for (; *str; str++) {
+		if (!crud(*str))
+			return 1;
+	}
+	return 0;
+}
+
 /*
  * Copy over a string to the destination, but avoid special
  * characters ('\n', '<' and '>') and remove crud at the end
@@ -389,6 +398,8 @@ const char *fmt_ident(const char *name, const char *email,
 			pw = xgetpwuid_self(NULL);
 			name = pw->pw_name;
 		}
+		if (strict && !has_non_crud(name))
+			die(_("name consists only of disallowed characters: %s"), name);
 	}
 
 	strbuf_reset(&ident);
diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh
index 6c057afc11..667f110f59 100755
--- a/t/t7518-ident-corner-cases.sh
+++ b/t/t7518-ident-corner-cases.sh
@@ -17,4 +17,9 @@ test_expect_success 'empty name and missing email' '
 	)
 '
 
+test_expect_success 'commit rejects all-crud name' '
+	test_must_fail env GIT_AUTHOR_NAME=" .;<>" \
+		git commit --allow-empty -m foo
+'
+
 test_done

From 94425552f308946456bb7823d0a1dd72ebd30bdd Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Thu, 23 Feb 2017 03:17:08 -0500
Subject: [PATCH 4/4] ident: do not ignore empty config name/email

When we read user.name and user.email from a config file,
they go into strbufs. When a caller asks ident_default_name()
for the value, we fallback to auto-detecting if the strbuf
is empty.

That means that explicitly setting an empty string in the
config is identical to not setting it at all. This is
potentially confusing, as we usually accept a configured
value as the final value.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 ident.c                       |  4 ++--
 t/t7518-ident-corner-cases.sh | 11 +++++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/ident.c b/ident.c
index ead09ff7f6..c0364fe3a1 100644
--- a/ident.c
+++ b/ident.c
@@ -153,7 +153,7 @@ static void copy_email(const struct passwd *pw, struct strbuf *email,
 
 const char *ident_default_name(void)
 {
-	if (!git_default_name.len) {
+	if (!(ident_config_given & IDENT_NAME_GIVEN) && !git_default_name.len) {
 		copy_gecos(xgetpwuid_self(&default_name_is_bogus), &git_default_name);
 		strbuf_trim(&git_default_name);
 	}
@@ -162,7 +162,7 @@ const char *ident_default_name(void)
 
 const char *ident_default_email(void)
 {
-	if (!git_default_email.len) {
+	if (!(ident_config_given & IDENT_MAIL_GIVEN) && !git_default_email.len) {
 		const char *email = getenv("EMAIL");
 
 		if (email && email[0]) {
diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh
index 667f110f59..b22f631261 100755
--- a/t/t7518-ident-corner-cases.sh
+++ b/t/t7518-ident-corner-cases.sh
@@ -22,4 +22,15 @@ test_expect_success 'commit rejects all-crud name' '
 		git commit --allow-empty -m foo
 '
 
+# We must test the actual error message here, as an unwanted
+# auto-detection could fail for other reasons.
+test_expect_success 'empty configured name does not auto-detect' '
+	(
+		sane_unset GIT_AUTHOR_NAME &&
+		test_must_fail \
+			git -c user.name= commit --allow-empty -m foo 2>err &&
+		test_i18ngrep "empty ident name" err
+	)
+'
+
 test_done