Skip to contentSkip to conversation

daemon.c: replace inet_ntop with getnameinfo

PATCH/RFC Benjamin Kramer 6 participants 8 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

git daemon's interpolated paths didn't support IPv6. Every IPv6 address was being converted to `0.0.0.0'.

Fix this by replacing inet_ntop(3) with the protocol agnostic getnameinfo(3) API.

---

With this patch we'll have colons in the per-IP directories for IPv6 addresses. Creating files with a : in the name fails on some OSes (e.g. Windows).

Is this OK for git or do we need to special case IPv6 addresses?

 daemon.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
daemon.c +3 -5
index daa4c8e..339d7ab 100644
@@ -446,17 +446,15 @@ static void parse_extra_args(char *extra_args, int buflen)
 		struct addrinfo hints;
 		struct addrinfo *ai;
 		int gai;
-		static char addrbuf[HOST_NAME_MAX + 1];
+		static char addrbuf[NI_MAXHOST];
 
 		memset(&hints, 0, sizeof(hints));
 		hints.ai_flags = AI_CANONNAME;
 
 		gai = getaddrinfo(hostname, 0, &hints, &ai);
 		if (!gai) {
-			struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
-
-			inet_ntop(AF_INET, &sin_addr->sin_addr,
-				  addrbuf, sizeof(addrbuf));
+			getnameinfo(ai->ai_addr, ai->ai_addrlen, addrbuf,
+				    sizeof(addrbuf), NULL, 0, NI_NUMERICHOST);
 			free(ip_address);
 			ip_address = xstrdup(addrbuf);
 
Signature · 1 line
1.6.3.1.g882bf
#1 lore raw
Quoted 6 lines · Benjamin Kramer

git daemon's interpolated paths didn't support IPv6. Every IPv6 address was being converted to `0.0.0.0'.

Fix this by replacing inet_ntop(3) with the protocol agnostic getnameinfo(3) API.

Quoted 7 lines

---

With this patch we'll have colons in the per-IP directories for IPv6 addresses. Creating files with a : in the name fails on some OSes (e.g. Windows).

What per-IP directories are you talking about?

Signature · 10 lines
Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Register now for Nordic Meet on Nagios, June 3-4 in Stockholm
http://nordicmeetonnagios.op5.org/

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
#2 lore raw
Quoted 1 line · Andreas Ericsson

What per-IP directories are you talking about?

git daemon has a feature called interpolated paths

If git daemon is started like this:

    git daemon --interpolated-path=%IP/%D
(the machine has two IPs: 123.123.123.123 (v4) and 2001:db8::1 (v6))
and someone clones a repository:
    git clone git://123.123.123.123/frotz

git daemon will look for the repository in the directory `123.123.123.123/frotz'

But if git daemon listens on the IPv6 interface and someone clones a repository:

    git clone git://2001:db8::1/frotz

Then git daemon will look for the repository in `0.0.0.0/frotz'

My patch makes it converting IPv6 addresses properly and if you the clone in my previous example it'll now look in `2001:db8::1/frotz' (with colons in the directory name)

I hope my intentions are now a bit clearer ;)

#3 lore raw
Quoted 12 lines · Benjamin Kramer

git daemon has a feature called interpolated paths

If git daemon is started like this:

    git daemon --interpolated-path=%IP/%D
(the machine has two IPs: 123.123.123.123 (v4) and 2001:db8::1 (v6))
and someone clones a repository:
    git clone git://123.123.123.123/frotz

git daemon will look for the repository in the directory `123.123.123.123/frotz'

But if git daemon listens on the IPv6 interface and someone clones a repository:

Quoted 7 lines
    git clone git://2001:db8::1/frotz

Then git daemon will look for the repository in `0.0.0.0/frotz'

My patch makes it converting IPv6 addresses properly and if you the clone in my previous example it'll now look in `2001:db8::1/frotz' (with colons in the
directory name)

I don't particularly care about git-daemon on Windows at this time because we don't build it anyway. But others have already had limited success, and they might care since getnameinfo() is not available. If we did have IPv6 support on Windows, we would indeed have troubles with those path names.

But even on non-Windows, a directory name with colons does not look kosher to me. Don't they look like PATH values? Or like remote addresses? Are IPv6 addresses used in this way by other software?

Moreover, I think that since IPv6 addresses can have at most one '::' abbreviation, but not in an unambiguous way, users of path-interpolation of IPv6 addresses are at the mercy of whether and how getnameinfo() makes use of '::'.

-- Hannes

#4 lore raw
Quoted 5 lines · Johannes Sixt

I don't particularly care about git-daemon on Windows at this time because we don't build it anyway. But others have already had limited success, and they might care since getnameinfo() is not available. If we did have IPv6 support on Windows, we would indeed have troubles with those path names.

The getnameinfo(3) code is in a #ifndef NO_IPV6 block anyways, so it won't hurt non-ipv6 builds. afaik getnameinfo(3) is available when getaddrinfo(3) is and that seems to be the case on newer windows versions.

Quoted 8 lines

But even on non-Windows, a directory name with colons does not look kosher to me. Don't they look like PATH values? Or like remote addresses? Are IPv6 addresses used in this way by other software?

Moreover, I think that since IPv6 addresses can have at most one '::' abbreviation, but not in an unambiguous way, users of path-interpolation of IPv6 addresses are at the mercy of whether and how getnameinfo() makes use of '::'.

I did a quick test with apache's VirtualDocumentRootIP and it looks like they are using :: only when it's unambigous. And yes, they use colons in the file name.

::1 stays ::1

2001:db8::abab:abab:0:abab:abab becomes 2001:db8:0:abab:abab:0:abab:abab

I don't know if they also use colons on windows because I don't have a windows box with IPv6 to test.

-->8-->8--

httpd.conf:
VirtualDocumentRootIP /foo/bar/%0

now point your browser to http://[::1]/ and watch your logs.

#5 lore raw
In reply to Benjamin Kramer
Quoted 14 lines · Benjamin Kramer
Quoted 1 line · Andreas Ericsson

What per-IP directories are you talking about?

git daemon has a feature called interpolated paths

If git daemon is started like this:

     git daemon --interpolated-path=%IP/%D
(the machine has two IPs: 123.123.123.123 (v4) and 2001:db8::1 (v6))
and someone clones a repository:
     git clone git://123.123.123.123/frotz

git daemon will look for the repository in the directory `123.123.123.123/frotz'

Quoted 8 lines

But if git daemon listens on the IPv6 interface and someone clones a repository:

     git clone git://2001:db8::1/frotz

Then git daemon will look for the repository in `0.0.0.0/frotz'

My patch makes it converting IPv6 addresses properly and if you the clone in my previous example it'll now look in `2001:db8::1/frotz' (with colons in the directory name)

BTW. this is not only MS Windows that have problems with ':' in paths (because of it being drive letter separator), but also IIRC MacOS X, where ':' and not '/' is directory separator.

The fact that ':' is separator of paths in $PATH environmental variable is a bit complication, but you can always escape ':' in $PATH.

Signature · 3 lines
Jakub Narebski
Poland
ShadeHawk on #git
#6 lore raw
Quoted 6 lines · Jakub Narebski

BTW. this is not only MS Windows that have problems with ':' in paths (because of it being drive letter separator), but also IIRC MacOS X, where ':' and not '/' is directory separator.

The fact that ':' is separator of paths in $PATH environmental variable is a bit complication, but you can always escape ':' in $PATH.

No, that was in "classic mac os"; osx uses "/".

-Miles

Signature · 2 lines
Politics, n. A strife of interests masquerading as a contest of
principles. The conduct of public affairs for private advantage.
#7 lore raw
In reply to Jakub Narebski
Quoted 3 lines · Jakub Narebski

BTW. this is not only MS Windows that have problems with ':' in paths (because of it being drive letter separator), but also IIRC MacOS X, where ':' and not '/' is directory separator.

Just FYI:

: was the directory separator in Mac OS 1-9. OS X uses / like most sane people. Finder will stop you from creating files or directories with : in the name, but the following works just fine from Terminal.app:

$ uname -a
Darwin Laptop.local 9.6.0 Darwin Kernel Version 9.6.0: Mon Nov 24 17:37:00 PST 2008; root:xnu-1228.9.59~1/RELEASE_I386 i386
$ touch test:test
$ ls test*
test:test
$ rm test:test
$

~~ Brian