From 862e80a413e45d34834ecd573e5c6b39e38ba850 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 23 Feb 2017 03:13:53 -0500 Subject: 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 Signed-off-by: Junio C Hamano --- t/t7518-ident-corner-cases.sh | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100755 t/t7518-ident-corner-cases.sh (limited to 't') 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 -- cgit v1.2.3 From 13b9a24e58f736b70e48846cf7e5b7cfa66c3fec Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 23 Feb 2017 03:15:55 -0500 Subject: 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 Signed-off-by: Junio C Hamano --- t/t7518-ident-corner-cases.sh | 5 +++++ 1 file changed, 5 insertions(+) (limited to 't') 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 -- cgit v1.2.3 From 94425552f308946456bb7823d0a1dd72ebd30bdd Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 23 Feb 2017 03:17:08 -0500 Subject: 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 Signed-off-by: Junio C Hamano --- t/t7518-ident-corner-cases.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 't') 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 -- cgit v1.2.3