Eliminate differences between Inet6AddressImpl_getLocalHostName() and Inet4AddressImpl_getLocalHostName()

[Volker Simonis]

Hi Michael,

thanks for your comments. Please find my answers inline:

On Fri, Oct 10, 2014 at 5:17 PM, Michael McMahon
<michael.x.mcmahon at 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
>>
>>
>