[PATCH/RFC] daemon.c: replace inet_ntop with getnameinfo

[PATCH/RFC] daemon.c: replace inet_ntop with getnameinfo

[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.

Signed-off-by: Benjamin Kramer <benny.kra@googlemail.com>
---

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(-)

diff --git a/daemon.c b/daemon.c
index daa4c8e..339d7ab 100644
--- a/daemon.c
+++ b/daemon.c
@@ -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);
 
-- 
1.6.3.1.g882bf

Re: [PATCH/RFC] daemon.c: replace inet_ntop with getnameinfo

[Andreas Ericsson]

Benjamin Kramer wrote:
> 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.
> 
> Signed-off-by: Benjamin Kramer <benny.kra@googlemail.com>
> ---
> 
> 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?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
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.

Re: [PATCH/RFC] daemon.c: replace inet_ntop with getnameinfo

[Benjamin Kramer]

Andreas Ericsson wrote:

> 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 ;)

Re: [PATCH/RFC] daemon.c: replace inet_ntop with getnameinfo

[Johannes Sixt]

Benjamin Kramer schrieb:
> 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 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

Re: [PATCH/RFC] daemon.c: replace inet_ntop with getnameinfo

[Benjamin Kramer]

Johannes Sixt schrieb:
> 
> 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.

> 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.

Re: [PATCH/RFC] daemon.c: replace inet_ntop with getnameinfo

[Jakub Narebski]

Benjamin Kramer <benny.kra@googlemail.com> writes:

> Andreas Ericsson wrote:
> 
> > 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)

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.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH/RFC] daemon.c: replace inet_ntop with getnameinfo

[Brian Gernhardt]

On May 7, 2009, at 9:34 AM, Jakub Narebski wrote:

> 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

Re: [PATCH/RFC] daemon.c: replace inet_ntop with getnameinfo

[Miles Bader]

Jakub Narebski <jnareb@gmail.com> writes:
> 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

-- 
Politics, n. A strife of interests masquerading as a contest of
principles. The conduct of public affairs for private advantage.