[PATCH] ident: handle NULL ai_canonname
- From
Jeff King <peff@peff.net>
- Date
- Sep 23, 2016, 04:37 UTC
- Message-ID
- <20160923043753.mfmiarneqc5rxgt3@sigill.intra.peff.net>
- In-Reply-To
- <20160923040730.76stbefz2ivyfy45@sigill.intra.peff.net>
On Fri, Sep 23, 2016 at 12:07:30AM -0400, Jeff King wrote:
> I have access to an OS X system, but if I understand the bug correctly, > reproducing it may involve re-setting the system hostname, which is not > something I'll be able to do. But I'll give it a shot.
Actually, it turned out to be pretty simple to reproduce (after reading 3e8a00a that John found, anyway; hooray for detailed commit messages). We just have to fake the output of gethostname(), but that is easily done since we can modify git's source. :)
So I was able to reproduce the bug, and indeed, the patch I posted fixes it. Here it is with a commit message.
Jonas, I'd be curious to know what the output of "hostname" is on your system.
-- >8 -- Subject: [PATCH] ident: handle NULL ai_canonname
We call getaddrinfo() to try to convert a short hostname into a fully-qualified one (to use it as an email domain). If there isn't a canonical name, getaddrinfo() will generally return either a NULL addrinfo list, or one in which ai->ai_canonname is a copy of the original name.
However, if the result of gethostname() looks like an IP address, then getaddrinfo() behaves differently on some systems. On OS X, it will return a "struct addrinfo" with a NULL ai_canonname, and we segfault feeding it to strchr().
This is hard to test reliably because it involves not only a system where we we have to fallback to gethostname() to come up with an ident, but also where the hostname is a number with no dots. But I was able to replicate the bug by faking a hostname, like:
diff --git a/ident.c b/ident.c
index e20a772..b790d28 100644
--- a/ident.c
+++ b/ident.c
@@ -128,6 +128,7 @@ static void add_domainname(struct strbuf *out, int *is_bogus)
*is_bogus = 1;
return;
}
+ xsnprintf(buf, sizeof(buf), "1");
if (strchr(buf, '.'))
strbuf_addstr(out, buf);
else if (canonical_name(buf, out) < 0) {and running "git var GIT_AUTHOR_IDENT" on an OS X system.
Before this patch it segfaults, and after we correctly complain of the bogus "user@1.(none)" address (though this bogus address would be suitable for non-object uses like writing reflogs).
Reported-by: Jonas Thiel <jonas.lierschied@gmx.de> Diagnosed-by: John Keeping <john@keeping.me.uk> Signed-off-by: Jeff King <peff@peff.net> --- ident.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ident.c b/ident.c index e20a772..d17b5bd 100644 --- a/ident.c +++ b/ident.c @@ -101,7 +101,7 @@ static int canonical_name(const char *host, struct strbuf *out) memset (&hints, '\0', sizeof (hints)); hints.ai_flags = AI_CANONNAME; if (!getaddrinfo(host, NULL, &hints, &ai)) { - if (ai && strchr(ai->ai_canonname, '.')) { + if (ai && ai->ai_canonname && strchr(ai->ai_canonname, '.')) { strbuf_addstr(out, ai->ai_canonname); status = 0; }
-- 2.10.0.482.gae5a597