[RFC/PATCH 0/6] git: please honor DNS SRV records

[RFC/PATCH 0/6] git: please honor DNS SRV records

[Jonathan Nieder]

Hi,

Julien Cristau wrote:
> On Tue, May 24, 2011 at 15:22:55 -0500, Jonathan Nieder wrote:

>> As rfc2782 explains, SRV records allow administrators "to use serveral
>> servers for a single domain, to move services from host to host with
>> little fuss, and to designate some hosts as primary servers for a
>> service and others as backups".  Julien Cristau noticed that in the
>> recent alioth move that second part would have been very handy (since
>> the git protocol doesn't include a concept of redirects).
[...]
> I played with this a bit tonight, came up with
> http://people.debian.org/~jcristau/git-srv-hack.diff which seems to work
> for me.  In case somebody wants to test/polish it up...

So I have played with it a little more.  Very rough, completely
untested, in desperate need of a test script, but I think it is ready
for some feedback.  To recap, this is really just an evil workaround
for the lack of support for redirects in git protocol, but it has a
nice side effect of making load balancing a little easier for server
administrators.

Patches 1-3 are code movement, to give the code that is being changed
a home and make it a little easier to contemplate.  In particular
patch 3 draws attention to some code from git daemon that makes DNS
lookups to get the canonical hostname and IP a client contacted.
Should it pay attention to SRV records?

Patch 5 is another cleanup patch.  It unifies the ipv4 and ipv6
implementations of git_tcp_connect by abstracting away some of the
differences between the underlying OS interfaces.  Patch 4 is a tiny
bugfix noticed in the process.

None of the above should be necessary for or even has much to do with
patch 6, the nominal topic of the series.  Sorry about that.

Anyway, patch 6 is an attempt to implement rfc2782, expanded from the
implementation of a subset that Julien sent.  It doesn't pay attention
to the "additional data" section of the SRV response to avoid A or
AAAA lookups, but it probably should.

At the back of my head there is this nagging feeling that getaddrinfo
should accept an ai_flags bit to do this for us automatically, but it
doesn't as far as I can tell.  Maybe this can be a stepping stone
toward that...

If nothing else, it's kind of fun.  Thoughts?  Does the general idea
seem sane?  Am I overlooking some standard function that takes care of
this all automatically?

[PATCH 1/6] transport: expose git_tcp_connect and friends in new tcp.h

[Jonathan Nieder]

Before adding functionality to them, let's move the functions
git_tcp_connect and git_proxy_connect used to resolve a hostname and
connect to it to a separate tcp.c file.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile  |    2 +
 connect.c |  270 +------------------------------------------------------------
 tcp.c     |  269 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tcp.h     |    8 ++
 4 files changed, 280 insertions(+), 269 deletions(-)
 create mode 100644 tcp.c
 create mode 100644 tcp.h

diff --git a/Makefile b/Makefile
index e40ac0c..fa3a47f 100644
--- a/Makefile
+++ b/Makefile
@@ -666,6 +666,7 @@ LIB_OBJS += string-list.o
 LIB_OBJS += submodule.o
 LIB_OBJS += symlinks.o
 LIB_OBJS += tag.o
+LIB_OBJS += tcp.o
 LIB_OBJS += trace.o
 LIB_OBJS += transport.o
 LIB_OBJS += transport-helper.o
@@ -1958,6 +1959,7 @@ builtin/prune.o builtin/reflog.o reachable.o: reachable.h
 builtin/commit.o builtin/revert.o wt-status.o: wt-status.h
 builtin/tar-tree.o archive-tar.o: tar.h
 connect.o transport.o http-backend.o: url.h
+connect.o tcp.o: tcp.h
 http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
 http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h
 
diff --git a/connect.c b/connect.c
index 2119c3f..9dafcc8 100644
--- a/connect.c
+++ b/connect.c
@@ -5,6 +5,7 @@
 #include "refs.h"
 #include "run-command.h"
 #include "remote.h"
+#include "tcp.h"
 #include "url.h"
 
 static char *server_capabilities;
@@ -150,275 +151,6 @@ static enum protocol get_protocol(const char *name)
 	die("I don't handle protocol '%s'", name);
 }
 
-#define STR_(s)	# s
-#define STR(s)	STR_(s)
-
-static void get_host_and_port(char **host, const char **port)
-{
-	char *colon, *end;
-
-	if (*host[0] == '[') {
-		end = strchr(*host + 1, ']');
-		if (end) {
-			*end = 0;
-			end++;
-			(*host)++;
-		} else
-			end = *host;
-	} else
-		end = *host;
-	colon = strchr(end, ':');
-
-	if (colon) {
-		*colon = 0;
-		*port = colon + 1;
-	}
-}
-
-#ifndef NO_IPV6
-
-static const char *ai_name(const struct addrinfo *ai)
-{
-	static char addr[NI_MAXHOST];
-	if (getnameinfo(ai->ai_addr, ai->ai_addrlen, addr, sizeof(addr), NULL, 0,
-			NI_NUMERICHOST) != 0)
-		strcpy(addr, "(unknown)");
-
-	return addr;
-}
-
-/*
- * Returns a connected socket() fd, or else die()s.
- */
-static int git_tcp_connect_sock(char *host, int flags)
-{
-	int sockfd = -1, saved_errno = 0;
-	const char *port = STR(DEFAULT_GIT_PORT);
-	struct addrinfo hints, *ai0, *ai;
-	int gai;
-	int cnt = 0;
-
-	get_host_and_port(&host, &port);
-	if (!*port)
-		port = "<none>";
-
-	memset(&hints, 0, sizeof(hints));
-	hints.ai_socktype = SOCK_STREAM;
-	hints.ai_protocol = IPPROTO_TCP;
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "Looking up %s ... ", host);
-
-	gai = getaddrinfo(host, port, &hints, &ai);
-	if (gai)
-		die("Unable to look up %s (port %s) (%s)", host, port, gai_strerror(gai));
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
-
-	for (ai0 = ai; ai; ai = ai->ai_next) {
-		sockfd = socket(ai->ai_family,
-				ai->ai_socktype, ai->ai_protocol);
-		if (sockfd < 0) {
-			saved_errno = errno;
-			continue;
-		}
-		if (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {
-			saved_errno = errno;
-			fprintf(stderr, "%s[%d: %s]: errno=%s\n",
-				host,
-				cnt,
-				ai_name(ai),
-				strerror(saved_errno));
-			close(sockfd);
-			sockfd = -1;
-			continue;
-		}
-		if (flags & CONNECT_VERBOSE)
-			fprintf(stderr, "%s ", ai_name(ai));
-		break;
-	}
-
-	freeaddrinfo(ai0);
-
-	if (sockfd < 0)
-		die("unable to connect a socket (%s)", strerror(saved_errno));
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\n");
-
-	return sockfd;
-}
-
-#else /* NO_IPV6 */
-
-/*
- * Returns a connected socket() fd, or else die()s.
- */
-static int git_tcp_connect_sock(char *host, int flags)
-{
-	int sockfd = -1, saved_errno = 0;
-	const char *port = STR(DEFAULT_GIT_PORT);
-	char *ep;
-	struct hostent *he;
-	struct sockaddr_in sa;
-	char **ap;
-	unsigned int nport;
-	int cnt;
-
-	get_host_and_port(&host, &port);
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "Looking up %s ... ", host);
-
-	he = gethostbyname(host);
-	if (!he)
-		die("Unable to look up %s (%s)", host, hstrerror(h_errno));
-	nport = strtoul(port, &ep, 10);
-	if ( ep == port || *ep ) {
-		/* Not numeric */
-		struct servent *se = getservbyname(port,"tcp");
-		if ( !se )
-			die("Unknown port %s", port);
-		nport = se->s_port;
-	}
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
-
-	for (cnt = 0, ap = he->h_addr_list; *ap; ap++, cnt++) {
-		sockfd = socket(he->h_addrtype, SOCK_STREAM, 0);
-		if (sockfd < 0) {
-			saved_errno = errno;
-			continue;
-		}
-
-		memset(&sa, 0, sizeof sa);
-		sa.sin_family = he->h_addrtype;
-		sa.sin_port = htons(nport);
-		memcpy(&sa.sin_addr, *ap, he->h_length);
-
-		if (connect(sockfd, (struct sockaddr *)&sa, sizeof sa) < 0) {
-			saved_errno = errno;
-			fprintf(stderr, "%s[%d: %s]: errno=%s\n",
-				host,
-				cnt,
-				inet_ntoa(*(struct in_addr *)&sa.sin_addr),
-				strerror(saved_errno));
-			close(sockfd);
-			sockfd = -1;
-			continue;
-		}
-		if (flags & CONNECT_VERBOSE)
-			fprintf(stderr, "%s ",
-				inet_ntoa(*(struct in_addr *)&sa.sin_addr));
-		break;
-	}
-
-	if (sockfd < 0)
-		die("unable to connect a socket (%s)", strerror(saved_errno));
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\n");
-
-	return sockfd;
-}
-
-#endif /* NO_IPV6 */
-
-
-static void git_tcp_connect(int fd[2], char *host, int flags)
-{
-	int sockfd = git_tcp_connect_sock(host, flags);
-
-	fd[0] = sockfd;
-	fd[1] = dup(sockfd);
-}
-
-
-static char *git_proxy_command;
-
-static int git_proxy_command_options(const char *var, const char *value,
-		void *cb)
-{
-	if (!strcmp(var, "core.gitproxy")) {
-		const char *for_pos;
-		int matchlen = -1;
-		int hostlen;
-		const char *rhost_name = cb;
-		int rhost_len = strlen(rhost_name);
-
-		if (git_proxy_command)
-			return 0;
-		if (!value)
-			return config_error_nonbool(var);
-		/* [core]
-		 * ;# matches www.kernel.org as well
-		 * gitproxy = netcatter-1 for kernel.org
-		 * gitproxy = netcatter-2 for sample.xz
-		 * gitproxy = netcatter-default
-		 */
-		for_pos = strstr(value, " for ");
-		if (!for_pos)
-			/* matches everybody */
-			matchlen = strlen(value);
-		else {
-			hostlen = strlen(for_pos + 5);
-			if (rhost_len < hostlen)
-				matchlen = -1;
-			else if (!strncmp(for_pos + 5,
-					  rhost_name + rhost_len - hostlen,
-					  hostlen) &&
-				 ((rhost_len == hostlen) ||
-				  rhost_name[rhost_len - hostlen -1] == '.'))
-				matchlen = for_pos - value;
-			else
-				matchlen = -1;
-		}
-		if (0 <= matchlen) {
-			/* core.gitproxy = none for kernel.org */
-			if (matchlen == 4 &&
-			    !memcmp(value, "none", 4))
-				matchlen = 0;
-			git_proxy_command = xmemdupz(value, matchlen);
-		}
-		return 0;
-	}
-
-	return git_default_config(var, value, cb);
-}
-
-static int git_use_proxy(const char *host)
-{
-	git_proxy_command = getenv("GIT_PROXY_COMMAND");
-	git_config(git_proxy_command_options, (void*)host);
-	return (git_proxy_command && *git_proxy_command);
-}
-
-static struct child_process *git_proxy_connect(int fd[2], char *host)
-{
-	const char *port = STR(DEFAULT_GIT_PORT);
-	const char **argv;
-	struct child_process *proxy;
-
-	get_host_and_port(&host, &port);
-
-	argv = xmalloc(sizeof(*argv) * 4);
-	argv[0] = git_proxy_command;
-	argv[1] = host;
-	argv[2] = port;
-	argv[3] = NULL;
-	proxy = xcalloc(1, sizeof(*proxy));
-	proxy->argv = argv;
-	proxy->in = -1;
-	proxy->out = -1;
-	if (start_command(proxy))
-		die("cannot start proxy %s", argv[0]);
-	fd[0] = proxy->out; /* read from proxy stdout */
-	fd[1] = proxy->in;  /* write to proxy stdin */
-	return proxy;
-}
-
 #define MAX_CMD_LEN 1024
 
 static char *get_port(char *host)
diff --git a/tcp.c b/tcp.c
new file mode 100644
index 0000000..89f5c62
--- /dev/null
+++ b/tcp.c
@@ -0,0 +1,269 @@
+#include "cache.h"
+#include "run-command.h"
+
+#define STR_(s)	# s
+#define STR(s)	STR_(s)
+
+static char *git_proxy_command;
+
+static int git_proxy_command_options(const char *var, const char *value,
+		void *cb)
+{
+	if (!strcmp(var, "core.gitproxy")) {
+		const char *for_pos;
+		int matchlen = -1;
+		int hostlen;
+		const char *rhost_name = cb;
+		int rhost_len = strlen(rhost_name);
+
+		if (git_proxy_command)
+			return 0;
+		if (!value)
+			return config_error_nonbool(var);
+		/* [core]
+		 * ;# matches www.kernel.org as well
+		 * gitproxy = netcatter-1 for kernel.org
+		 * gitproxy = netcatter-2 for sample.xz
+		 * gitproxy = netcatter-default
+		 */
+		for_pos = strstr(value, " for ");
+		if (!for_pos)
+			/* matches everybody */
+			matchlen = strlen(value);
+		else {
+			hostlen = strlen(for_pos + 5);
+			if (rhost_len < hostlen)
+				matchlen = -1;
+			else if (!strncmp(for_pos + 5,
+					  rhost_name + rhost_len - hostlen,
+					  hostlen) &&
+				 ((rhost_len == hostlen) ||
+				  rhost_name[rhost_len - hostlen -1] == '.'))
+				matchlen = for_pos - value;
+			else
+				matchlen = -1;
+		}
+		if (0 <= matchlen) {
+			/* core.gitproxy = none for kernel.org */
+			if (matchlen == 4 &&
+			    !memcmp(value, "none", 4))
+				matchlen = 0;
+			git_proxy_command = xmemdupz(value, matchlen);
+		}
+		return 0;
+	}
+
+	return git_default_config(var, value, cb);
+}
+
+int git_use_proxy(const char *host)
+{
+	git_proxy_command = getenv("GIT_PROXY_COMMAND");
+	git_config(git_proxy_command_options, (void*)host);
+	return (git_proxy_command && *git_proxy_command);
+}
+
+static void get_host_and_port(char **host, const char **port)
+{
+	char *colon, *end;
+
+	if (*host[0] == '[') {
+		end = strchr(*host + 1, ']');
+		if (end) {
+			*end = 0;
+			end++;
+			(*host)++;
+		} else
+			end = *host;
+	} else
+		end = *host;
+	colon = strchr(end, ':');
+
+	if (colon) {
+		*colon = 0;
+		*port = colon + 1;
+	}
+}
+
+#ifndef NO_IPV6
+
+static const char *ai_name(const struct addrinfo *ai)
+{
+	static char addr[NI_MAXHOST];
+	if (getnameinfo(ai->ai_addr, ai->ai_addrlen, addr, sizeof(addr), NULL, 0,
+			NI_NUMERICHOST) != 0)
+		strcpy(addr, "(unknown)");
+
+	return addr;
+}
+
+/*
+ * Returns a connected socket() fd, or else die()s.
+ */
+static int git_tcp_connect_sock(char *host, int flags)
+{
+	int sockfd = -1, saved_errno = 0;
+	const char *port = STR(DEFAULT_GIT_PORT);
+	struct addrinfo hints, *ai0, *ai;
+	int gai;
+	int cnt = 0;
+
+	get_host_and_port(&host, &port);
+	if (!*port)
+		port = "<none>";
+
+	memset(&hints, 0, sizeof(hints));
+	hints.ai_socktype = SOCK_STREAM;
+	hints.ai_protocol = IPPROTO_TCP;
+
+	if (flags & CONNECT_VERBOSE)
+		fprintf(stderr, "Looking up %s ... ", host);
+
+	gai = getaddrinfo(host, port, &hints, &ai);
+	if (gai)
+		die("Unable to look up %s (port %s) (%s)", host, port, gai_strerror(gai));
+
+	if (flags & CONNECT_VERBOSE)
+		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
+
+	for (ai0 = ai; ai; ai = ai->ai_next) {
+		sockfd = socket(ai->ai_family,
+				ai->ai_socktype, ai->ai_protocol);
+		if (sockfd < 0) {
+			saved_errno = errno;
+			continue;
+		}
+		if (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {
+			saved_errno = errno;
+			fprintf(stderr, "%s[%d: %s]: errno=%s\n",
+				host,
+				cnt,
+				ai_name(ai),
+				strerror(saved_errno));
+			close(sockfd);
+			sockfd = -1;
+			continue;
+		}
+		if (flags & CONNECT_VERBOSE)
+			fprintf(stderr, "%s ", ai_name(ai));
+		break;
+	}
+
+	freeaddrinfo(ai0);
+
+	if (sockfd < 0)
+		die("unable to connect a socket (%s)", strerror(saved_errno));
+
+	if (flags & CONNECT_VERBOSE)
+		fprintf(stderr, "done.\n");
+
+	return sockfd;
+}
+
+#else /* NO_IPV6 */
+
+/*
+ * Returns a connected socket() fd, or else die()s.
+ */
+static int git_tcp_connect_sock(char *host, int flags)
+{
+	int sockfd = -1, saved_errno = 0;
+	const char *port = STR(DEFAULT_GIT_PORT);
+	char *ep;
+	struct hostent *he;
+	struct sockaddr_in sa;
+	char **ap;
+	unsigned int nport;
+	int cnt;
+
+	get_host_and_port(&host, &port);
+
+	if (flags & CONNECT_VERBOSE)
+		fprintf(stderr, "Looking up %s ... ", host);
+
+	he = gethostbyname(host);
+	if (!he)
+		die("Unable to look up %s (%s)", host, hstrerror(h_errno));
+	nport = strtoul(port, &ep, 10);
+	if ( ep == port || *ep ) {
+		/* Not numeric */
+		struct servent *se = getservbyname(port,"tcp");
+		if ( !se )
+			die("Unknown port %s", port);
+		nport = se->s_port;
+	}
+
+	if (flags & CONNECT_VERBOSE)
+		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
+
+	for (cnt = 0, ap = he->h_addr_list; *ap; ap++, cnt++) {
+		sockfd = socket(he->h_addrtype, SOCK_STREAM, 0);
+		if (sockfd < 0) {
+			saved_errno = errno;
+			continue;
+		}
+
+		memset(&sa, 0, sizeof sa);
+		sa.sin_family = he->h_addrtype;
+		sa.sin_port = htons(nport);
+		memcpy(&sa.sin_addr, *ap, he->h_length);
+
+		if (connect(sockfd, (struct sockaddr *)&sa, sizeof sa) < 0) {
+			saved_errno = errno;
+			fprintf(stderr, "%s[%d: %s]: errno=%s\n",
+				host,
+				cnt,
+				inet_ntoa(*(struct in_addr *)&sa.sin_addr),
+				strerror(saved_errno));
+			close(sockfd);
+			sockfd = -1;
+			continue;
+		}
+		if (flags & CONNECT_VERBOSE)
+			fprintf(stderr, "%s ",
+				inet_ntoa(*(struct in_addr *)&sa.sin_addr));
+		break;
+	}
+
+	if (sockfd < 0)
+		die("unable to connect a socket (%s)", strerror(saved_errno));
+
+	if (flags & CONNECT_VERBOSE)
+		fprintf(stderr, "done.\n");
+
+	return sockfd;
+}
+
+#endif /* NO_IPV6 */
+
+void git_tcp_connect(int fd[2], char *host, int flags)
+{
+	int sockfd = git_tcp_connect_sock(host, flags);
+
+	fd[0] = sockfd;
+	fd[1] = dup(sockfd);
+}
+
+struct child_process *git_proxy_connect(int fd[2], char *host)
+{
+	const char *port = STR(DEFAULT_GIT_PORT);
+	const char **argv;
+	struct child_process *proxy;
+
+	get_host_and_port(&host, &port);
+
+	argv = xmalloc(sizeof(*argv) * 4);
+	argv[0] = git_proxy_command;
+	argv[1] = host;
+	argv[2] = port;
+	argv[3] = NULL;
+	proxy = xcalloc(1, sizeof(*proxy));
+	proxy->argv = argv;
+	proxy->in = -1;
+	proxy->out = -1;
+	if (start_command(proxy))
+		die("cannot start proxy %s", argv[0]);
+	fd[0] = proxy->out; /* read from proxy stdout */
+	fd[1] = proxy->in;  /* write to proxy stdin */
+	return proxy;
+}
diff --git a/tcp.h b/tcp.h
new file mode 100644
index 0000000..4de5f71
--- /dev/null
+++ b/tcp.h
@@ -0,0 +1,8 @@
+#ifndef TCP_H
+#define TCP_H
+
+extern int git_use_proxy(const char *host);
+extern void git_tcp_connect(int fd[2], char *host, int flags);
+extern struct child_process *git_proxy_connect(int fd[2], char *host);
+
+#endif
-- 
1.7.5.3

[PATCH 2/6] daemon: make host resolution into a separate function

[Jonathan Nieder]

For each incoming connection, the git daemon runs a DNS query using
the client-supplied hostname parameter, to determine the canonical
hostname and IP address in case it is needed by the
--interpolated-path feature (to distinguish between virtual hosts).
Put the code for this in a separate function so the details of this
lookup can be easily tweaked.

Signed-off-by; Jonathan Nieder <jrnieder@gmail.com>
---
 daemon.c |  112 +++++++++++++++++++++++++++++++++++---------------------------
 1 files changed, 63 insertions(+), 49 deletions(-)

diff --git a/daemon.c b/daemon.c
index 4c8346d..3958cb6 100644
--- a/daemon.c
+++ b/daemon.c
@@ -432,6 +432,65 @@ static void parse_host_and_port(char *hostport, char **host,
 	}
 }
 
+#ifndef NO_IPV6
+
+static void locate_host(const char *hostname, char **ip_address,
+						char **canon_hostname)
+{
+	struct addrinfo hints;
+	struct addrinfo *ai;
+	int gai;
+	static char addrbuf[HOST_NAME_MAX + 1];
+	struct sockaddr_in *sin_addr;
+
+	memset(&hints, 0, sizeof(hints));
+	hints.ai_flags = AI_CANONNAME;
+
+	gai = getaddrinfo(hostname, NULL, &hints, &ai);
+	if (gai)
+		return;
+
+	sin_addr = (void *)ai->ai_addr;
+	inet_ntop(AF_INET, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));
+	free(*ip_address);
+	*ip_address = xstrdup(addrbuf);
+
+	free(*canon_hostname);
+	*canon_hostname = xstrdup(ai->ai_canonname ?
+				  ai->ai_canonname : *ip_address);
+
+	freeaddrinfo(ai);
+}
+
+#else
+
+static void locate_host(const char *hostname, char **ip_address,
+						char **canon_hostname)
+{
+	struct hostent *hent;
+	struct sockaddr_in sa;
+	char **ap;
+	static char addrbuf[HOST_NAME_MAX + 1];
+
+	hent = gethostbyname(hostname);
+
+	ap = hent->h_addr_list;
+	memset(&sa, 0, sizeof sa);
+	sa.sin_family = hent->h_addrtype;
+	sa.sin_port = htons(0);
+	memcpy(&sa.sin_addr, *ap, hent->h_length);
+
+	inet_ntop(hent->h_addrtype, &sa.sin_addr,
+		  addrbuf, sizeof(addrbuf));
+
+	free(*canon_hostname);
+	*canon_hostname = xstrdup(hent->h_name);
+	free(*ip_address);
+	*ip_address = xstrdup(addrbuf);
+}
+
+#endif
+
 /*
  * Read the host as supplied by the client connection.
  */
@@ -467,56 +526,11 @@ static void parse_host_arg(char *extra_args, int buflen)
 	}
 
 	/*
-	 * Locate canonical hostname and its IP address.
+	 * Locate canonical hostname and its IP address,
+	 * if possible.
 	 */
-	if (hostname) {
-#ifndef NO_IPV6
-		struct addrinfo hints;
-		struct addrinfo *ai;
-		int gai;
-		static char addrbuf[HOST_NAME_MAX + 1];
-
-		memset(&hints, 0, sizeof(hints));
-		hints.ai_flags = AI_CANONNAME;
-
-		gai = getaddrinfo(hostname, NULL, &hints, &ai);
-		if (!gai) {
-			struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
-
-			inet_ntop(AF_INET, &sin_addr->sin_addr,
-				  addrbuf, sizeof(addrbuf));
-			free(ip_address);
-			ip_address = xstrdup(addrbuf);
-
-			free(canon_hostname);
-			canon_hostname = xstrdup(ai->ai_canonname ?
-						 ai->ai_canonname : ip_address);
-
-			freeaddrinfo(ai);
-		}
-#else
-		struct hostent *hent;
-		struct sockaddr_in sa;
-		char **ap;
-		static char addrbuf[HOST_NAME_MAX + 1];
-
-		hent = gethostbyname(hostname);
-
-		ap = hent->h_addr_list;
-		memset(&sa, 0, sizeof sa);
-		sa.sin_family = hent->h_addrtype;
-		sa.sin_port = htons(0);
-		memcpy(&sa.sin_addr, *ap, hent->h_length);
-
-		inet_ntop(hent->h_addrtype, &sa.sin_addr,
-			  addrbuf, sizeof(addrbuf));
-
-		free(canon_hostname);
-		canon_hostname = xstrdup(hent->h_name);
-		free(ip_address);
-		ip_address = xstrdup(addrbuf);
-#endif
-	}
+	if (hostname)
+		locate_host(hostname, &ip_address, &canon_hostname);
 }
 
 
-- 
1.7.5.3

[PATCH 3/6] daemon: move locate_host to tcp.c

[Jonathan Nieder]

Keep the different name resolution functions close together so they
can learn from each other and perhaps share code in the future.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile |    2 +-
 daemon.c |   62 ++------------------------------------------------------------
 tcp.c    |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tcp.h    |    3 +++
 4 files changed, 59 insertions(+), 61 deletions(-)

diff --git a/Makefile b/Makefile
index fa3a47f..df84157 100644
--- a/Makefile
+++ b/Makefile
@@ -1959,7 +1959,7 @@ builtin/prune.o builtin/reflog.o reachable.o: reachable.h
 builtin/commit.o builtin/revert.o wt-status.o: wt-status.h
 builtin/tar-tree.o archive-tar.o: tar.h
 connect.o transport.o http-backend.o: url.h
-connect.o tcp.o: tcp.h
+connect.o daemon.o tcp.o: tcp.h
 http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
 http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h
 
diff --git a/daemon.c b/daemon.c
index 3958cb6..3261616 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "pkt-line.h"
+#include "tcp.h"
 #include "exec_cmd.h"
 #include "run-command.h"
 #include "strbuf.h"
@@ -432,65 +433,6 @@ static void parse_host_and_port(char *hostport, char **host,
 	}
 }
 
-#ifndef NO_IPV6
-
-static void locate_host(const char *hostname, char **ip_address,
-						char **canon_hostname)
-{
-	struct addrinfo hints;
-	struct addrinfo *ai;
-	int gai;
-	static char addrbuf[HOST_NAME_MAX + 1];
-	struct sockaddr_in *sin_addr;
-
-	memset(&hints, 0, sizeof(hints));
-	hints.ai_flags = AI_CANONNAME;
-
-	gai = getaddrinfo(hostname, NULL, &hints, &ai);
-	if (gai)
-		return;
-
-	sin_addr = (void *)ai->ai_addr;
-	inet_ntop(AF_INET, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));
-	free(*ip_address);
-	*ip_address = xstrdup(addrbuf);
-
-	free(*canon_hostname);
-	*canon_hostname = xstrdup(ai->ai_canonname ?
-				  ai->ai_canonname : *ip_address);
-
-	freeaddrinfo(ai);
-}
-
-#else
-
-static void locate_host(const char *hostname, char **ip_address,
-						char **canon_hostname)
-{
-	struct hostent *hent;
-	struct sockaddr_in sa;
-	char **ap;
-	static char addrbuf[HOST_NAME_MAX + 1];
-
-	hent = gethostbyname(hostname);
-
-	ap = hent->h_addr_list;
-	memset(&sa, 0, sizeof sa);
-	sa.sin_family = hent->h_addrtype;
-	sa.sin_port = htons(0);
-	memcpy(&sa.sin_addr, *ap, hent->h_length);
-
-	inet_ntop(hent->h_addrtype, &sa.sin_addr,
-		  addrbuf, sizeof(addrbuf));
-
-	free(*canon_hostname);
-	*canon_hostname = xstrdup(hent->h_name);
-	free(*ip_address);
-	*ip_address = xstrdup(addrbuf);
-}
-
-#endif
-
 /*
  * Read the host as supplied by the client connection.
  */
@@ -530,7 +472,7 @@ static void parse_host_arg(char *extra_args, int buflen)
 	 * if possible.
 	 */
 	if (hostname)
-		locate_host(hostname, &ip_address, &canon_hostname);
+		git_locate_host(hostname, &ip_address, &canon_hostname);
 }
 
 
diff --git a/tcp.c b/tcp.c
index 89f5c62..2cb90db 100644
--- a/tcp.c
+++ b/tcp.c
@@ -97,6 +97,34 @@ static const char *ai_name(const struct addrinfo *ai)
 	return addr;
 }
 
+void git_locate_host(const char *hostname, char **ip_address,
+					char **canon_hostname)
+{
+	struct addrinfo hints;
+	struct addrinfo *ai;
+	int gai;
+	static char addrbuf[HOST_NAME_MAX + 1];
+	struct sockaddr_in *sin_addr;
+
+	memset(&hints, 0, sizeof(hints));
+	hints.ai_flags = AI_CANONNAME;
+
+	gai = getaddrinfo(hostname, NULL, &hints, &ai);
+	if (gai)
+		return;
+
+	sin_addr = (void *)ai->ai_addr;
+	inet_ntop(AF_INET, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));
+	free(*ip_address);
+	*ip_address = xstrdup(addrbuf);
+
+	free(*canon_hostname);
+	*canon_hostname = xstrdup(ai->ai_canonname ?
+				  ai->ai_canonname : *ip_address);
+
+	freeaddrinfo(ai);
+}
+
 /*
  * Returns a connected socket() fd, or else die()s.
  */
@@ -162,6 +190,31 @@ static int git_tcp_connect_sock(char *host, int flags)
 
 #else /* NO_IPV6 */
 
+void git_locate_host(const char *hostname, char **ip_address,
+					char **canon_hostname)
+{
+	struct hostent *hent;
+	struct sockaddr_in sa;
+	char **ap;
+	static char addrbuf[HOST_NAME_MAX + 1];
+
+	hent = gethostbyname(hostname);
+
+	ap = hent->h_addr_list;
+	memset(&sa, 0, sizeof sa);
+	sa.sin_family = hent->h_addrtype;
+	sa.sin_port = htons(0);
+	memcpy(&sa.sin_addr, *ap, hent->h_length);
+
+	inet_ntop(hent->h_addrtype, &sa.sin_addr,
+		  addrbuf, sizeof(addrbuf));
+
+	free(*canon_hostname);
+	*canon_hostname = xstrdup(hent->h_name);
+	free(*ip_address);
+	*ip_address = xstrdup(addrbuf);
+}
+
 /*
  * Returns a connected socket() fd, or else die()s.
  */
diff --git a/tcp.h b/tcp.h
index 4de5f71..bed3cdc 100644
--- a/tcp.h
+++ b/tcp.h
@@ -1,6 +1,9 @@
 #ifndef TCP_H
 #define TCP_H
 
+extern void git_locate_host(const char *hostname,
+			char **ip_address, char **canon_hostname);
+
 extern int git_use_proxy(const char *host);
 extern void git_tcp_connect(int fd[2], char *host, int flags);
 extern struct child_process *git_proxy_connect(int fd[2], char *host);
-- 
1.7.5.3

[PATCH 4/6] transport: fix index in ipv6 connection failed message

[Jonathan Nieder]

Ever since v1.5.3-rc0~185^2~1 (Verbose connect messages to show
the IP addresses used, 2007-05-23), git's ipv4 connection failure
messages have looked something like

	git.example.com[2: 127.0.0.1]: errno=Network Unreachable

The increasing index ("2" above) indicates which item in the list of
addresses returned by gethostbyname was being tried so the operator
can tell that git was not just trying to resolve the same hostname
again and again.

The ipv6 codepath prints something similar, but the counter is never
incremented so the index is always "0".  Fix it.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 tcp.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tcp.c b/tcp.c
index 2cb90db..9916272 100644
--- a/tcp.c
+++ b/tcp.c
@@ -134,7 +134,7 @@ static int git_tcp_connect_sock(char *host, int flags)
 	const char *port = STR(DEFAULT_GIT_PORT);
 	struct addrinfo hints, *ai0, *ai;
 	int gai;
-	int cnt = 0;
+	int cnt;
 
 	get_host_and_port(&host, &port);
 	if (!*port)
@@ -154,7 +154,7 @@ static int git_tcp_connect_sock(char *host, int flags)
 	if (flags & CONNECT_VERBOSE)
 		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
 
-	for (ai0 = ai; ai; ai = ai->ai_next) {
+	for (cnt = 0, ai0 = ai; ai; ai = ai->ai_next, cnt++) {
 		sockfd = socket(ai->ai_family,
 				ai->ai_socktype, ai->ai_protocol);
 		if (sockfd < 0) {
-- 
1.7.5.3

[PATCH 5/6] tcp: unify ipv4 and ipv6 code paths

[Jonathan Nieder]

The new DNS API abstracts away differences between the gethostbyname-
and getaddrinfo-centric interfaces for looking up a host, making the
code to use them in connect.c a little easier to read.

To make a lookup:

	resolver_result ai;
	dns_resolve(host, port, 0, &ai);
	...
	dns_free(ai);

To iterate over responses:

	resolved_address i;
	for_each_address(i, ai) {
		...
	}

In the !NO_IPV6 codepath, the git_locate_host function used to find
the canonical IP and hostname for a git server's public address (for
virtual hosting) tells getaddrinfo to restrict attention to TCP
services after this patch.  That should make a difference because the
service parameter is NULL.

No functional change intended.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile   |    5 ++
 dns-ipv4.c |   33 +++++++++++
 dns-ipv4.h |   65 ++++++++++++++++++++++
 dns-ipv6.c |   45 +++++++++++++++
 dns-ipv6.h |   31 ++++++++++
 tcp.c      |  180 +++++++++++-------------------------------------------------
 6 files changed, 211 insertions(+), 148 deletions(-)
 create mode 100644 dns-ipv4.c
 create mode 100644 dns-ipv4.h
 create mode 100644 dns-ipv6.c
 create mode 100644 dns-ipv6.h

diff --git a/Makefile b/Makefile
index df84157..7d3cb41 100644
--- a/Makefile
+++ b/Makefile
@@ -1474,6 +1474,11 @@ ifdef NO_TRUSTABLE_FILEMODE
 endif
 ifdef NO_IPV6
 	BASIC_CFLAGS += -DNO_IPV6
+	LIB_OBJS += dns-ipv4.o
+	LIB_H += dns-ipv4.h
+else
+	LIB_OBJS += dns-ipv6.o
+	LIB_H += dns-ipv6.h
 endif
 ifdef NO_UINTMAX_T
 	BASIC_CFLAGS += -Duintmax_t=uint32_t
diff --git a/dns-ipv4.c b/dns-ipv4.c
new file mode 100644
index 0000000..39849c3
--- /dev/null
+++ b/dns-ipv4.c
@@ -0,0 +1,33 @@
+#include "cache.h"
+#include "dns-ipv4.h"
+
+int dns_resolve(const char *host, const char *port, int flags,
+		resolver_result *res)
+{
+	char *ep;
+	struct hostent *he;
+	unsigned int nport;
+
+	he = gethostbyname(host);
+	if (!he && (flags & RESOLVE_IGNORE_ERROR))
+		return -1;
+	if (!he)
+		die("Unable to look up %s (%s)", host, hstrerror(h_errno));
+
+	if (!port) {
+		nport = 0;
+		goto done;
+	}
+	nport = strtoul(port, &ep, 10);
+	if ( ep == port || *ep ) {
+		/* Not numeric */
+		struct servent *se = getservbyname(port,"tcp");
+		if ( !se )
+			die("Unknown port %s", port);
+		nport = se->s_port;
+	}
+done:
+	res->he = he;
+	res->port = nport;
+	return 0;
+}
diff --git a/dns-ipv4.h b/dns-ipv4.h
new file mode 100644
index 0000000..ef7208f
--- /dev/null
+++ b/dns-ipv4.h
@@ -0,0 +1,65 @@
+#ifndef DNS_IPV4_H
+#define DNS_IPV4_H
+
+struct ipv4_address {
+	char **ap;
+	struct sockaddr_in sa;
+};
+
+struct ipv4_addrinfo {
+	struct hostent *he;
+	unsigned int port;
+};
+
+typedef struct ipv4_addrinfo resolver_result;
+typedef struct ipv4_address resolved_address;
+
+enum {
+	RESOLVE_CANONNAME = 1,
+	RESOLVE_IGNORE_ERROR = 2
+};
+extern int dns_resolve(const char *host, const char *port, int flags,
+			resolver_result *res);
+
+static inline const char *dns_name(const resolved_address *addr)
+{
+	return inet_ntoa(*(struct in_addr *)&addr->sa.sin_addr);
+}
+
+static inline char *dns_ip_address(const resolved_address *addr,
+					const resolver_result *ai)
+{
+	char addrbuf[HOST_NAME_MAX + 1];
+	inet_ntop(ai->he->h_addrtype, &addr->sa.sin_addr,
+		  addrbuf, sizeof(addrbuf));
+	return xstrdup(addrbuf);
+}
+
+static inline int dns_fill_sockaddr_(char *ap,
+		const struct ipv4_addrinfo *ai, struct sockaddr_in *sa)
+{
+	if (!ap)	/* done. */
+		return -1;
+
+	memset(sa, 0, sizeof(*sa));
+	sa->sin_family = ai->he->h_addrtype;
+	sa->sin_port = htons(ai->port);
+	memcpy(&sa->sin_addr, ap, ai->he->h_length);
+	return 0;
+}
+
+#define for_each_address(addr, ai) \
+	for ((addr).ap = (ai).he->h_addr_list; \
+	     !dns_fill_sockaddr_(*(addr).ap, &(ai), &(addr).sa); \
+	     (addr).ap++)
+
+#define dns_family(addr, ai) ((ai).he->h_addrtype)
+#define dns_socktype(addr, ai) SOCK_STREAM
+#define dns_protocol(addr, ai) 0
+#define dns_addr(addr, ai) ((struct sockaddr *) &(addr).sa)
+#define dns_addrlen(addr, ai) sizeof((addr).sa)
+#define dns_canonname(addr, ai) ((ai).he->h_name)
+
+#define dns_free(ai) do { /* nothing */ } while (0)
+
+#endif
diff --git a/dns-ipv6.c b/dns-ipv6.c
new file mode 100644
index 0000000..b5f298a
--- /dev/null
+++ b/dns-ipv6.c
@@ -0,0 +1,45 @@
+#include "cache.h"
+#include "dns-ipv6.h"
+
+const char *dns_name(const resolved_address *i)
+{
+	const struct addrinfo *ai = *i;
+	static char addr[NI_MAXHOST];
+	if (getnameinfo(ai->ai_addr, ai->ai_addrlen, addr, sizeof(addr), NULL, 0,
+			NI_NUMERICHOST) != 0)
+		strcpy(addr, "(unknown)");
+
+	return addr;
+}
+
+char *dns_ip_address(const resolved_address *i, const resolver_result *ai0)
+{
+	const struct addrinfo *ai = *i;
+	char addrbuf[HOST_NAME_MAX + 1];
+	struct sockaddr_in *sin_addr;
+
+	sin_addr = (void *)ai->ai_addr;
+	inet_ntop(AF_INET, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));
+	return xstrdup(addrbuf);
+}
+
+int dns_resolve(const char *host, const char *port, int flags,
+		resolver_result *res)
+{
+	struct addrinfo hints;
+	int gai;
+
+	memset(&hints, 0, sizeof(hints));
+	if (flags & RESOLVE_CANONNAME)
+		hints.ai_flags = AI_CANONNAME;
+	hints.ai_socktype = SOCK_STREAM;
+	hints.ai_protocol = IPPROTO_TCP;
+
+	gai = getaddrinfo(host, port, &hints, res);
+	if (gai && (flags & RESOLVE_IGNORE_ERROR))
+		return -1;
+	if (gai)
+		die("Unable to look up %s (port %s) (%s)", host, port, gai_strerror(gai));
+
+	return 0;
+}
diff --git a/dns-ipv6.h b/dns-ipv6.h
new file mode 100644
index 0000000..339877f
--- /dev/null
+++ b/dns-ipv6.h
@@ -0,0 +1,31 @@
+#ifndef DNS_IPV6_H
+#define DNS_IPV6_H
+
+typedef struct addrinfo *resolver_result;
+typedef const struct addrinfo *resolved_address;
+
+enum {
+	RESOLVE_CANONNAME = 1,
+	RESOLVE_IGNORE_ERROR = 2
+};
+extern int dns_resolve(const char *host, const char *port, int flags,
+			resolver_result *res);
+/* result is in static buffer */
+extern const char *dns_name(const resolved_address *i);
+/* result is in malloc'ed buffer */
+extern char *dns_ip_address(const resolved_address *i,
+				const resolver_result *ai);
+
+#define for_each_address(i, ai) \
+	for (i = ai; i; i = (i)->ai_next)
+
+#define dns_family(i, ai) ((i)->ai_family)
+#define dns_socktype(i, ai) ((i)->ai_socktype)
+#define dns_protocol(i, ai) ((i)->ai_protocol)
+#define dns_addr(i, ai) ((i)->ai_addr)
+#define dns_addrlen(i, ai) ((i)->ai_addrlen)
+#define dns_canonname(i, ai) ((i)->ai_canonname)
+
+#define dns_free(ai) freeaddrinfo(ai)
+
+#endif
diff --git a/tcp.c b/tcp.c
index 9916272..aacc2d1 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1,6 +1,12 @@
 #include "cache.h"
 #include "run-command.h"
 
+#ifndef NO_IPV6
+#include "dns-ipv6.h"
+#else
+#include "dns-ipv4.h"
+#endif
+
 #define STR_(s)	# s
 #define STR(s)	STR_(s)
 
@@ -85,44 +91,27 @@ static void get_host_and_port(char **host, const char **port)
 	}
 }
 
-#ifndef NO_IPV6
-
-static const char *ai_name(const struct addrinfo *ai)
-{
-	static char addr[NI_MAXHOST];
-	if (getnameinfo(ai->ai_addr, ai->ai_addrlen, addr, sizeof(addr), NULL, 0,
-			NI_NUMERICHOST) != 0)
-		strcpy(addr, "(unknown)");
-
-	return addr;
-}
-
 void git_locate_host(const char *hostname, char **ip_address,
 					char **canon_hostname)
 {
-	struct addrinfo hints;
-	struct addrinfo *ai;
-	int gai;
-	static char addrbuf[HOST_NAME_MAX + 1];
-	struct sockaddr_in *sin_addr;
+	resolver_result ai;
+	resolved_address i;
 
-	memset(&hints, 0, sizeof(hints));
-	hints.ai_flags = AI_CANONNAME;
-
-	gai = getaddrinfo(hostname, NULL, &hints, &ai);
-	if (gai)
+	if (dns_resolve(hostname, NULL,
+			RESOLVE_CANONNAME | RESOLVE_IGNORE_ERROR, &ai))
 		return;
 
-	sin_addr = (void *)ai->ai_addr;
-	inet_ntop(AF_INET, &sin_addr->sin_addr, addrbuf, sizeof(addrbuf));
-	free(*ip_address);
-	*ip_address = xstrdup(addrbuf);
+	for_each_address(i, ai) {
+		free(*ip_address);
+		*ip_address = dns_ip_address(&i, &ai);
 
-	free(*canon_hostname);
-	*canon_hostname = xstrdup(ai->ai_canonname ?
-				  ai->ai_canonname : *ip_address);
+		free(*canon_hostname);
+		*canon_hostname = xstrdup(dns_canonname(i, ai) ?
+					dns_canonname(i, ai) : *ip_address);
+		break;
+	}
 
-	freeaddrinfo(ai);
+	dns_free(ai);
 }
 
 /*
@@ -132,52 +121,48 @@ static int git_tcp_connect_sock(char *host, int flags)
 {
 	int sockfd = -1, saved_errno = 0;
 	const char *port = STR(DEFAULT_GIT_PORT);
-	struct addrinfo hints, *ai0, *ai;
-	int gai;
-	int cnt;
+	resolver_result ai;
+	resolved_address i;
+	int cnt = -1;
 
 	get_host_and_port(&host, &port);
 	if (!*port)
 		port = "<none>";
 
-	memset(&hints, 0, sizeof(hints));
-	hints.ai_socktype = SOCK_STREAM;
-	hints.ai_protocol = IPPROTO_TCP;
-
 	if (flags & CONNECT_VERBOSE)
 		fprintf(stderr, "Looking up %s ... ", host);
 
-	gai = getaddrinfo(host, port, &hints, &ai);
-	if (gai)
-		die("Unable to look up %s (port %s) (%s)", host, port, gai_strerror(gai));
+	if (dns_resolve(host, port, 0, &ai))
+		die("BUG: dns_resolve returned error?");
 
 	if (flags & CONNECT_VERBOSE)
 		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
 
-	for (cnt = 0, ai0 = ai; ai; ai = ai->ai_next, cnt++) {
-		sockfd = socket(ai->ai_family,
-				ai->ai_socktype, ai->ai_protocol);
+	for_each_address(i, ai) {
+		cnt++;
+		sockfd = socket(dns_family(i, ai),
+				dns_socktype(i, ai), dns_protocol(i, ai));
 		if (sockfd < 0) {
 			saved_errno = errno;
 			continue;
 		}
-		if (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {
+		if (connect(sockfd, dns_addr(i, ai), dns_addrlen(i, ai)) < 0) {
 			saved_errno = errno;
 			fprintf(stderr, "%s[%d: %s]: errno=%s\n",
 				host,
 				cnt,
-				ai_name(ai),
+				dns_name(&i),
 				strerror(saved_errno));
 			close(sockfd);
 			sockfd = -1;
 			continue;
 		}
 		if (flags & CONNECT_VERBOSE)
-			fprintf(stderr, "%s ", ai_name(ai));
+			fprintf(stderr, "%s ", dns_name(&i));
 		break;
 	}
 
-	freeaddrinfo(ai0);
+	dns_free(ai);
 
 	if (sockfd < 0)
 		die("unable to connect a socket (%s)", strerror(saved_errno));
@@ -188,107 +173,6 @@ static int git_tcp_connect_sock(char *host, int flags)
 	return sockfd;
 }
 
-#else /* NO_IPV6 */
-
-void git_locate_host(const char *hostname, char **ip_address,
-					char **canon_hostname)
-{
-	struct hostent *hent;
-	struct sockaddr_in sa;
-	char **ap;
-	static char addrbuf[HOST_NAME_MAX + 1];
-
-	hent = gethostbyname(hostname);
-
-	ap = hent->h_addr_list;
-	memset(&sa, 0, sizeof sa);
-	sa.sin_family = hent->h_addrtype;
-	sa.sin_port = htons(0);
-	memcpy(&sa.sin_addr, *ap, hent->h_length);
-
-	inet_ntop(hent->h_addrtype, &sa.sin_addr,
-		  addrbuf, sizeof(addrbuf));
-
-	free(*canon_hostname);
-	*canon_hostname = xstrdup(hent->h_name);
-	free(*ip_address);
-	*ip_address = xstrdup(addrbuf);
-}
-
-/*
- * Returns a connected socket() fd, or else die()s.
- */
-static int git_tcp_connect_sock(char *host, int flags)
-{
-	int sockfd = -1, saved_errno = 0;
-	const char *port = STR(DEFAULT_GIT_PORT);
-	char *ep;
-	struct hostent *he;
-	struct sockaddr_in sa;
-	char **ap;
-	unsigned int nport;
-	int cnt;
-
-	get_host_and_port(&host, &port);
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "Looking up %s ... ", host);
-
-	he = gethostbyname(host);
-	if (!he)
-		die("Unable to look up %s (%s)", host, hstrerror(h_errno));
-	nport = strtoul(port, &ep, 10);
-	if ( ep == port || *ep ) {
-		/* Not numeric */
-		struct servent *se = getservbyname(port,"tcp");
-		if ( !se )
-			die("Unknown port %s", port);
-		nport = se->s_port;
-	}
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
-
-	for (cnt = 0, ap = he->h_addr_list; *ap; ap++, cnt++) {
-		sockfd = socket(he->h_addrtype, SOCK_STREAM, 0);
-		if (sockfd < 0) {
-			saved_errno = errno;
-			continue;
-		}
-
-		memset(&sa, 0, sizeof sa);
-		sa.sin_family = he->h_addrtype;
-		sa.sin_port = htons(nport);
-		memcpy(&sa.sin_addr, *ap, he->h_length);
-
-		if (connect(sockfd, (struct sockaddr *)&sa, sizeof sa) < 0) {
-			saved_errno = errno;
-			fprintf(stderr, "%s[%d: %s]: errno=%s\n",
-				host,
-				cnt,
-				inet_ntoa(*(struct in_addr *)&sa.sin_addr),
-				strerror(saved_errno));
-			close(sockfd);
-			sockfd = -1;
-			continue;
-		}
-		if (flags & CONNECT_VERBOSE)
-			fprintf(stderr, "%s ",
-				inet_ntoa(*(struct in_addr *)&sa.sin_addr));
-		break;
-	}
-
-	if (sockfd < 0)
-		die("unable to connect a socket (%s)", strerror(saved_errno));
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\n");
-
-	return sockfd;
-}
-
-#endif /* NO_IPV6 */
-
 void git_tcp_connect(int fd[2], char *host, int flags)
 {
 	int sockfd = git_tcp_connect_sock(host, flags);
-- 
1.7.5.3

[PATCH 6/6] transport: learn to honor DNS SRV records

[Jonathan Nieder]

As rfc2782 explains, SRV records allow administrators "to use serveral
servers for a single domain, to move services from host to host with
little fuss, and to designate some hosts as primary servers for a
service and others as backups".  Someone noticed in a recent server
move that that second part would have been very handy (since the git
protocol doesn't include a concept of redirects).

Sadly, getaddrinfo itself does not include a facility to resolve SRV
records automatically.  So this patch uses the libbind functions from
libresolv to parse the response for ourselves.  Hopefully after
reading enough specimens of code like this, someone will be disgusted
enough to extend libc to take care of this in general.

RFC 2782 requires some non-determinism in the order of hosts
contacted; this patch uses drand48() for that.  To avoid causing
trouble for platforms without the libbind ns_* functions or drand48(),
the new SRV support is only built when requested with the USE_SRV_RR
compile-time option.

Based on a patch by Julien Cristau <jcristau@debian.org>.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Thanks for reading.

 Makefile |   10 ++
 srv.c    |  290 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 srv.h    |   15 +++
 tcp.c    |   90 +++++++++++++-------
 4 files changed, 375 insertions(+), 30 deletions(-)
 create mode 100644 srv.c
 create mode 100644 srv.h

diff --git a/Makefile b/Makefile
index 7d3cb41..316b29e 100644
--- a/Makefile
+++ b/Makefile
@@ -124,6 +124,10 @@ all::
 # Notably on Solaris hstrerror resides in libresolv and on Solaris 7
 # inet_ntop and inet_pton additionally reside there.
 #
+# Define USE_SRV_RR if you want git to pay attention to SRV resource records
+# when looking up servers to contact over git protocol.  This implies
+# NEEDS_RESOLV.
+#
 # Define NO_MMAP if you want to avoid mmap.
 #
 # Define NO_PTHREADS if you do not have or do not want to use Pthreads.
@@ -1357,6 +1361,11 @@ endif
 ifdef NEEDS_NSL
 	EXTLIBS += -lnsl
 endif
+ifdef USE_SRV_RR
+	BASIC_CFLAGS += -DUSE_SRV_RR
+	LIB_OBJS += srv.o
+	NEEDS_RESOLV = YesPlease
+endif
 ifdef NEEDS_RESOLV
 	EXTLIBS += -lresolv
 endif
@@ -1965,6 +1974,7 @@ builtin/commit.o builtin/revert.o wt-status.o: wt-status.h
 builtin/tar-tree.o archive-tar.o: tar.h
 connect.o transport.o http-backend.o: url.h
 connect.o daemon.o tcp.o: tcp.h
+tcp.o srv.o: srv.h
 http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
 http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h
 
diff --git a/srv.c b/srv.c
new file mode 100644
index 0000000..222c244
--- /dev/null
+++ b/srv.c
@@ -0,0 +1,290 @@
+#include "cache.h"
+#include "strbuf.h"
+#include "srv.h"
+
+#include <arpa/nameser.h>
+#include <resolv.h>
+
+struct parsed_srv_rr {
+	int priority;
+	int weight;
+	int port;
+	char *target;
+};
+
+static void srv_swap(struct parsed_srv_rr *p1, struct parsed_srv_rr *p2)
+{
+	char *a, *b;
+	int size = sizeof(struct parsed_srv_rr);
+
+	for (a = (char *) p1, b = (char *) p2; size; size--) {
+		char t = *a;
+		*a++ = *b;
+		*b++ = t;
+	}
+}
+
+static int priority_compare(const void *p1, const void *p2)
+{
+	const struct parsed_srv_rr *a = p1, *b = p2;
+
+	/* can't overflow because priorities are 16 bits wide */
+	return b->priority - a->priority;
+}
+
+static int get_qname_for_srv(struct strbuf *sb, const char *host)
+{
+	const char prefix[] = "_git._tcp.";
+	size_t hostlen;
+
+	hostlen = strlen(host);
+	if (unsigned_add_overflows(strlen(prefix) + 1, hostlen))
+		return -1;
+
+	strbuf_reset(sb);
+	strbuf_grow(sb, strlen(prefix) + hostlen);
+	strbuf_add(sb, prefix, strlen(prefix));
+	strbuf_add(sb, host, hostlen);
+	return 0;
+}
+
+static int srv_parse_rr(const ns_msg *msg,
+			const ns_rr *rr, struct parsed_srv_rr *res)
+{
+	const unsigned char *p;
+	char buf[1024];
+
+	if (ns_rr_rdlen(*rr) < 2+2+2 /* priority, weight, port */)
+		return -1;
+
+	p = ns_rr_rdata(*rr);
+	res->priority = *p++ << CHAR_BIT;
+	res->priority += *p++;
+
+	res->weight = *p++ << CHAR_BIT;
+	res->weight += *p++;
+
+	res->port = *p++ << CHAR_BIT;
+	res->port += *p++;
+
+	/*
+	 * RFC2782 doesn't allow compressed target domain names but we
+	 * might as well accept them anyway.
+	 */
+	if (dn_expand(ns_msg_base(*msg), ns_msg_end(*msg), p,
+			buf, sizeof(buf)) < 0)
+		return -1;
+
+	res->target = xstrdup(buf);
+	return 0;
+}
+
+static struct parsed_srv_rr *srv_parse(ns_msg *msg, int n)
+{
+	struct parsed_srv_rr *rrs;
+	int nr_parsed = 0;
+	int i;
+
+	rrs = xmalloc(sizeof(*rrs) * n);
+	for (i = 0; i < n; i++) {
+		ns_rr rr;
+
+		if (ns_parserr(msg, ns_s_an, i, &rr) ||
+		    srv_parse_rr(msg, &rr, rrs + i))
+			goto fail;
+		nr_parsed++;
+	}
+
+	return rrs;
+fail:
+	for (i = 0; i < nr_parsed; i++)
+		free(rrs[i].target);
+	free(rrs);
+	return NULL;
+}
+
+static int weighted_item(struct parsed_srv_rr *rrs, int n, unsigned int pos)
+{
+	int i;
+
+	for (i = 0; i < n; i++) {
+		unsigned int wt = rrs[i].weight;
+
+		if (wt >= pos)
+			break;
+		pos -= wt;
+	}
+	return i;
+}
+
+static void shuffle_one(struct parsed_srv_rr *rrs, int n,
+				int i, int *j, unsigned int *wt_remaining)
+{
+	unsigned int pos;
+	int k;
+
+	pos = (unsigned) (drand48() * (*wt_remaining + 1));
+
+	if (!pos) {
+		*wt_remaining -= rrs[i].weight;
+		return;
+	}
+
+	/* Which item will take the place of rrs[i]? */
+	if (*j < i)
+		*j = i;
+	k = *j + weighted_item(rrs + *j, n - *j, pos);
+
+	assert(k < n);
+	*wt_remaining -= rrs[k].weight;
+
+	if (k == i)
+		return;
+
+	srv_swap(rrs + i, rrs + k);
+
+	/*
+	 * If rrs[i] had weight zero, move it to stay in the clump
+	 * of weight-zero records.  rrs[k] cannot have had weight
+	 * zero because pos > 0.
+	 */
+	assert(*j <= k);
+	if (i < *j) {
+		srv_swap(rrs + k, rrs + *j + 1);
+		(*j)++;
+	}
+}
+
+static void weighted_shuffle(struct parsed_srv_rr *rrs, int n)
+{
+	int i, j;
+	unsigned int total = 0;
+	static int seeded;
+
+	/*
+	 * Calculate total weight and move weight-zero
+	 * records to the left.
+	 */
+	assert(n < (1 << 16));
+	for (i = j = 0; i < n; i++) {
+		unsigned int wt = rrs[i].weight;
+		assert(wt < (1 << 16));
+
+		if (!wt) {
+			srv_swap(rrs + i, rrs + j);
+			j++;
+		}
+
+		/*
+		 * In the worst case, n is 2^16 - 1 and
+		 * each weight is 2^16 - 1, making the total
+		 * a little less than 2^32.
+		 */
+		assert(!unsigned_add_overflows(total, wt + 1));
+		total += wt;
+	}
+	/* Usual case: all weights are zero. */
+	if (!total)
+		return;
+
+	if (!seeded) {
+		seeded = 1;
+		srand48(time(NULL));
+	}
+
+	for (i = 0; i < n; i++)
+		/*
+		 * Now the records starting at rrs[i] could be in any order,
+		 * except those of weight 0 are at the start of the list
+		 * (ending with rrs[j-1]).
+		 *
+		 * Pick an item at random, taking weights into account, and
+		 * make it rrs[i], preserving that invariant.
+		 */
+		shuffle_one(rrs, n, i, &j, &total);
+}
+
+static void sort_rrs(struct parsed_srv_rr *rrs, int n)
+{
+	int i, j, prio;
+
+	qsort(rrs, n, sizeof(*rrs), priority_compare);
+
+	/*
+	 * Within each priority level, order servers randomly,
+	 * respecting weight.
+	 */
+	j = 0;
+	prio = rrs[j].priority;
+	for (i = 0; i < n; i++) {
+		if (rrs[i].priority == prio)
+			continue;
+
+		weighted_shuffle(rrs + j, i - j);
+		j = i;
+		prio = rrs[j].priority;
+	}
+	weighted_shuffle(rrs + j, n - j);
+}
+
+/* Reference: RFC2782. */
+int get_srv(const char *host, struct host **hosts)
+{
+	struct strbuf sb = STRBUF_INIT;
+	unsigned char buf[1024];
+	ns_msg msg;
+	int len, n, i, ret;
+	struct parsed_srv_rr *rrs = NULL;
+	struct host *reply = NULL;
+
+	/* if no SRV record is found, fall back to plain address lookup */
+	ret = 0;
+
+	/* _git._tcp.<host> */
+	if (get_qname_for_srv(&sb, host))
+		goto out;
+	len = res_query(sb.buf, ns_c_in, ns_t_srv, buf, sizeof(buf));
+	if (len < 0)
+		goto out;
+
+	if (ns_initparse(buf, len, &msg))
+		goto out;
+	n = ns_msg_count(msg, ns_s_an);
+	if (!n)
+		goto out;
+	assert(n < (1 << 16));
+
+	/* Parse reply.  If a SRV RR cannot be parsed, give up. */
+	ret = -1;
+	rrs = srv_parse(&msg, n);
+	if (!rrs)
+		goto out;
+
+	/* A single RR with target "." means "go away". */
+	if (n == 1 &&
+	    (!*rrs[0].target || !strcmp(rrs[0].target, ".")))
+		goto out2;
+
+	sort_rrs(rrs, n);
+
+	/* Success! */
+	ret = n;
+	reply = xmalloc(n * sizeof(**hosts));
+	for (i = 0; i < n; i++) {
+		char buf[32];
+		snprintf(buf, sizeof(buf), "%d", rrs[i].port);
+
+		reply[i].hostname = rrs[i].target;
+		reply[i].port = xstrdup(buf);
+	}
+	*hosts = reply;
+	goto out;
+
+out2:
+	for (i = 0; i < n; i++)
+		free(rrs[i].target);
+out:
+	free(rrs);
+	strbuf_release(&sb);
+	return 0;
+}
diff --git a/srv.h b/srv.h
new file mode 100644
index 0000000..7cea4f4
--- /dev/null
+++ b/srv.h
@@ -0,0 +1,15 @@
+#ifndef SRV_H
+#define SRV_H
+
+struct host {
+	char *hostname;
+	char *port;
+};
+
+#ifndef USE_SRV_RR
+#define get_srv(host, hosts) 0
+#else
+extern int get_srv(const char *host, struct host **hosts);
+#endif
+
+#endif
diff --git a/tcp.c b/tcp.c
index aacc2d1..53ad2e0 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "run-command.h"
+#include "srv.h"
 
 #ifndef NO_IPV6
 #include "dns-ipv6.h"
@@ -120,49 +121,73 @@ void git_locate_host(const char *hostname, char **ip_address,
 static int git_tcp_connect_sock(char *host, int flags)
 {
 	int sockfd = -1, saved_errno = 0;
-	const char *port = STR(DEFAULT_GIT_PORT);
+	const char *port = NULL;
+	struct host *hosts = NULL;
 	resolver_result ai;
 	resolved_address i;
 	int cnt = -1;
+	int j, n = 0;
 
 	get_host_and_port(&host, &port);
+	if (!port) {
+		port = STR(DEFAULT_GIT_PORT);
+		n = get_srv(host, &hosts);
+	}
+	if (n < 0)
+		die("Unable to look up %s", host);
 	if (!*port)
 		port = "<none>";
+	if (!n) {
+		hosts = xmalloc(sizeof(*hosts));
+		hosts[0].hostname = xstrdup(host);
+		hosts[0].port = xstrdup(port);
+		n = 1;
+	}
 
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "Looking up %s ... ", host);
-
-	if (dns_resolve(host, port, 0, &ai))
-		die("BUG: dns_resolve returned error?");
-
-	if (flags & CONNECT_VERBOSE)
-		fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
+	for (j = 0; j < n; j++) {
+		if (flags & CONNECT_VERBOSE)
+			fprintf(stderr, "Looking up %s ... ", hosts[j].hostname);
 
-	for_each_address(i, ai) {
-		cnt++;
-		sockfd = socket(dns_family(i, ai),
-				dns_socktype(i, ai), dns_protocol(i, ai));
-		if (sockfd < 0) {
-			saved_errno = errno;
-			continue;
-		}
-		if (connect(sockfd, dns_addr(i, ai), dns_addrlen(i, ai)) < 0) {
-			saved_errno = errno;
-			fprintf(stderr, "%s[%d: %s]: errno=%s\n",
-				host,
-				cnt,
-				dns_name(&i),
-				strerror(saved_errno));
-			close(sockfd);
-			sockfd = -1;
+		if (dns_resolve(hosts[j].hostname,
+				hosts[j].port, RESOLVE_IGNORE_ERROR, &ai)) {
+			if (flags & CONNECT_VERBOSE)
+				fprintf(stderr, "failed.\n");
 			continue;
 		}
+
 		if (flags & CONNECT_VERBOSE)
-			fprintf(stderr, "%s ", dns_name(&i));
-		break;
-	}
+			fprintf(stderr, "done.\nConnecting to %s (port %s) ... ",
+					hosts[j].hostname, hosts[j].port);
+
+		for_each_address(i, ai) {
+			cnt++;
+			sockfd = socket(dns_family(i, ai),
+					dns_socktype(i, ai), dns_protocol(i, ai));
+			if (sockfd < 0) {
+				saved_errno = errno;
+				continue;
+			}
+			if (connect(sockfd, dns_addr(i, ai), dns_addrlen(i, ai)) < 0) {
+				saved_errno = errno;
+				fprintf(stderr, "%s[%d: %s]: errno=%s\n",
+					hosts[j].hostname,
+					cnt,
+					dns_name(&i),
+					strerror(saved_errno));
+				close(sockfd);
+				sockfd = -1;
+				continue;
+			}
+			if (flags & CONNECT_VERBOSE)
+				fprintf(stderr, "%s ", dns_name(&i));
+			break;
+		}
+
+		dns_free(ai);
 
-	dns_free(ai);
+		if (sockfd >= 0)
+			break;
+	}
 
 	if (sockfd < 0)
 		die("unable to connect a socket (%s)", strerror(saved_errno));
@@ -170,6 +195,11 @@ static int git_tcp_connect_sock(char *host, int flags)
 	if (flags & CONNECT_VERBOSE)
 		fprintf(stderr, "done.\n");
 
+	for (j = 0; j < n; j++) {
+		free(hosts[j].hostname);
+		free(hosts[j].port);
+	}
+	free(hosts);
 	return sockfd;
 }
 
-- 
1.7.5.3

Re: [RFC/PATCH 0/6] git: please honor DNS SRV records

[Jonathan Nieder]

Jonathan Nieder wrote:

> Does the general idea seem sane?

Here's a diffstat for the curious.

Jonathan Nieder (6):
  transport: expose git_tcp_connect and friends in new tcp.h
  daemon: make host resolution into a separate function
  daemon: move locate_host to tcp.c
  transport: fix index in ipv6 connection failed message
  tcp: unify ipv4 and ipv6 code paths
  transport: learn to honor DNS SRV records

 Makefile   |   17 ++++
 connect.c  |  270 +-------------------------------------------------------
 daemon.c   |   54 +----------
 dns-ipv4.c |   33 +++++++
 dns-ipv4.h |   65 ++++++++++++++
 dns-ipv6.c |   45 +++++++++
 dns-ipv6.h |   31 +++++++
 srv.c      |  290 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 srv.h      |   15 +++
 tcp.c      |  236 ++++++++++++++++++++++++++++++++++++++++++++++++
 tcp.h      |   11 +++
 11 files changed, 749 insertions(+), 318 deletions(-)
 create mode 100644 dns-ipv4.c
 create mode 100644 dns-ipv4.h
 create mode 100644 dns-ipv6.c
 create mode 100644 dns-ipv6.h
 create mode 100644 srv.c
 create mode 100644 srv.h
 create mode 100644 tcp.c
 create mode 100644 tcp.h

Re: [PATCH 5/6] tcp: unify ipv4 and ipv6 code paths

[Jonathan Nieder]

Jonathan Nieder wrote:

>                  tells getaddrinfo to restrict attention to TCP
> services after this patch.  That should make a difference because the
> service parameter is NULL.

Sorry, the last sentence should say "That should _not_ make a
difference".  You would be able to tell the difference between the
before and after with "strace git daemon" because of the ai_socktype
and ai_protocol settings:

[...]
> +int dns_resolve(const char *host, const char *port, int flags,
> +		resolver_result *res)
> +{
> +	struct addrinfo hints;
> +	int gai;
> +
> +	memset(&hints, 0, sizeof(hints));
> +	if (flags & RESOLVE_CANONNAME)
> +		hints.ai_flags = AI_CANONNAME;
> +	hints.ai_socktype = SOCK_STREAM;
> +	hints.ai_protocol = IPPROTO_TCP;
> +
> +	gai = getaddrinfo(host, port, &hints, res);

but I don't think the effect is any different --- the ai_socktype and
protocol hints are for getservbyname functionality and the code that
cares about it was already passing those hints.