Hi Michael, thanks for your comments. Please find my answers inline:
On Fri, Oct 10, 2014 at 5:17 PM, Michael McMahon <michael.x.mcma...@oracle.com> wrote: > Actually, I think you are probably right that the comment > about Solaris not returning a fqdn, is the reason why the reverse lookup was > done > and it is only doing the reverse lookup on Solaris and Linux (ipv4 only) > > The main concern I guess is just compatibility. There could be environments > that > depend on InetAddress.getLocalHost() returning a FQDN (eg Linux ipv4 only). > But the problem is that Linux currently doesn't return a FQDN (see my first mail). And anyway, that doesn't depend on how many times we're doing a reverse lookup but instead it only depends on how you have configured /etc/hosts and /etc/nsswitch.conf. And notice, that we are doing the reverse lookup twice in the ipv4 case: once in Inet4AddressImpl_getLocalHostName() and a second time in InetAddress.getLocalHost() before we call getHostName() on the resulting address. That seems unnecessary. > Given that Windows and (by default) Linux don't do this, not many people > would > likely be affected. So, maybe we could disable it by default, but allow it > to be switched > on by system property, with the same implementation for all platforms. > Using a property to switch the behaviour would be fine for me. But in MHO I think we should use a simple and common version on all platforms and update the API documentation of InetAddress.getLocalHost() to make it clear that there's no guarantee that it returns neither a FQDN nor a simple name. I'll post a first webrev tomorrow. Regards, Volker > Michael > > > On 10/10/14 15:03, Michael McMahon wrote: >> >> Hi Volker, >> >> I agree on the refactoring proposal. That would be a useful thing to do. >> On the second point. Removing getnameinfo() and using AI_CANONNAME in >> getaddrinfo() instead, would not be the exact equivalent however. >> >> getnameinfo() with AI_CANONNAME is taking the canonical host name as >> reported directly by the name service. Whereas the additional >> getnameinfo() >> is doing a reverse lookup of the forward looked up IP address. >> >> Having said that, I'm not sure I agree that step is really necessary. >> Before >> we change it, I'd like to check back through the history to see why >> exactly >> it was done and will report back. >> >> Thanks, >> Michael >> >> On 08/10/14 17:59, Volker Simonis wrote: >>> >>> Hi, >>> >>> I wonder why the implementations of >>> Inet6AddressImpl_ >>> getLocalHostName() and >>> Inet4AddressImpl_getLocalHostName() are different . It seems to me >>> that this is simply copy/paste error since the very first 1.4 version. >>> Here's what we currently have: >>> >>> Inet4AddressImpl_getLocalHostName() >>> >>> if (gethostname(hostname, NI_MAXHOST)) { >>> /* Something went wrong, maybe networking is not setup? */ >>> strcpy(hostname, "localhost"); >>> } else { >>> ... >>> error = getaddrinfo(hostname, NULL, &hints, &res); >>> ... >>> error = getnameinfo(res->ai_addr, res->ai_addrlen, hostname, >>> ... >>> } >>> >>> We first call gethostname(). If that succeeds we call getaddrinfo() >>> with the host name returned by gethostname() and if that succeeds >>> again we call getnameinfo() with the address returned by getaddrinfo() >>> to get the final hostname. This is uniformly done on all Unix >>> platforms. >>> >>> And here's how the corresponding IPv6 version looks like: >>> >>> Inet6AddressImpl_getLocalHostName() >>> >>> ret = gethostname(hostname, NI_MAXHOST); >>> >>> #if defined(__solaris__) && defined(AF_INET6) >>> ... >>> error = getaddrinfo(hostname, NULL, &hints, &res); >>> ... >>> error = getnameinfo(res->ai_addr, res->ai_addrlen, hostname, >>> #endif >>> >>> As you can see, for the IPv6 version we only do the >>> getaddrinfo()/getnameinfo() roundtrip on Solaris although there is no >>> evidence that the inovolved system calls behave differently for IPv4 >>> and IPv6. Instead I think the IPv6 version is just a leftover of the >>> very first implementation which hasn't been updated in the same way >>> like its IPv4 counterpart. Notice the comment in the IPv6 version >>> which says "Solaris doesn't want to give us a fully qualified domain >>> name". But that holds true for the Linux version as well - >>> gethostname() on Linux returns "uname -n" which is usually >>> unqualified. The comment further notices that even the reverse lookup >>> may not return a fully qualified name which is true because that's >>> highly system and name service dependent. >>> >>> I think we can therefore safely use the same implementation for both, >>> Inet4AddressImpl_getLocalHostName() and >>> Inet6AddressImpl_getLocalHostName(). We should actually refactor this >>> implementation into its own function to avoid differences in the >>> future. >>> >>> There's also a funny punchline in this whole story: trough a bug >>> introduced by the Mac OS port, the two implementations have been >>> semanticall equal for a while. But then this has been changed back as >>> a fix for "7166687: InetAddress.getLocalHost().getHostName() returns >>> FQDN " (https://bugs.openjdk.java.net/browse/JDK-7166687 , >>> http://hg.openjdk.java.net/jdk9/dev/jdk/rev/b26c04717735). But I'm >>> pretty sure that change didn't really fixed the problem described in >>> the bug report (maybe just incidentally worked around). Here's why: >>> >>> getLocalHostName() is used by InetAddress.getLocalHost(), but it's >>> result (i.e. the host name) is immediately converted back into an >>> address (i.e. InetAddress) on which subsequently getHostName() is >>> called. The result of getHostName() only depends on the available >>> name services and not on the fact if getLocalHostName() returned a >>> simple or a fully qualified host name. However the resolution of a >>> host name to an IP-address may be different depending on whether we >>> have a simple (may resolve trough /etc/hosts) or fully qualified name >>> (may resolve through name service like DNS). >>> >>> The hacky way to fix issue 7166687 would be to have the corresponding >>> entries in your /etc/hosts (i.e. short names for both, IPv4 and IPv6 >>> interfaces). The right way to handle it would be to actually expect >>> both, simple and full qualified host names as return values from >>> InetAddress.getHostName(). >>> >>> Notice the InetAddress.getLocalHost() isn't guaranteed to not return >>> the loopback address. If you have configured your system such that >>> /etc/hosts associates your host name with the loopback device and >>> /etc/resolve.conf in such a way to first query /etc/hosts before doing >>> a name service lookup (which is not unusual) than >>> InetAddress.getLocalHost() will do just that - return the address of >>> your loopback device! >>> >>> So to finally cut a long story short, I propose the following: >>> >>> - refactor the implementation of Inet6AddressImpl_getLocalHostName() >>> and Inet4AddressImpl_getLocalHostName() into a single function which >>> corresponds to the current Inet4AddressImpl_getLocalHostName() >>> implementation. >>> >>> - it may be also possible to complete omit the call to getnameinfo() >>> in that new implementation, because as far as I can see the >>> 'ai_canonname' field of the first addrinfo structure returned by >>> getaddrinfo() already contains the canonical name of the host if we >>> pass AI_CANONNAME as 'hints' to getaddrinfo (which we already do). >>> >>> What do you think? >>> >>> Regards, >>> Volker >> >> >