Re: Failure with libidn2 on OpenBSD.

[Simon Josefsson]

I wrote:

> These days I believe the general recommendation is to use system
> getaddrinfo() which has been updated to support IDNA on the platforms we
> care about, and linking directly to libidn/libidn2 does not really help
> except to trigger bugs.  Of course, we should test that only relying on
> getaddrinfo() works the same as our existing uses of libidn/libidn2
> though!  There are legitimate uses of calls to libidn/libidn2 so not
> everything should be removed.

Indeed, on a modern system with glibc, everything works via getaddrinfo
without need to link with libidn/libidn2.  The code in inetutils is
buggy today (it passes on non-domains to IDNA functions), and if anyone
wants IDN-enabled applications on non-GNU systems, the best place to fix
this would be in gnulib's getaddrinfo.  Then it would work automatically
in inetutils too.  Fixing the bugs and supporting the build complexity
just to cater for non-GNU platforms is not worth the maintainence cost
for us, IMHO.

I have tested ping/ping6 with the patch below, and it works:

jas@latte:~/src/inetutils/ping$ sudo ./ping6 räksmörgås.josefsson.org
PING räksmörgås.josefsson.org (2001:9b1:8633::102): 56 data bytes
./ping6: sending packet: Nätverket kan inte nås
jas@latte:~/src/inetutils/ping$ sudo ./ping räksmörgås.josefsson.org
PING räksmörgås.josefsson.org (178.174.241.102): 56 data bytes
64 bytes from 178.174.241.102: icmp_seq=0 ttl=55 time=2,324 ms
^C--- räksmörgås.josefsson.org ping statistics ---
1 packets transmitted, 1 packets received, 0% packet loss
round-trip min/avg/max/stddev = 2,324/2,324/2,324/0,000 ms
jas@latte:~/src/inetutils/ping$ 

The IPv6 failure is just because my laptop happened to be on a non-IPv6
network, but name resolution worked fine.

Given the discussion above, I propose to remove all uses of
libidn/libidn2 from inetutils, and place a note in gnulib's getaddrinfo
that it is missing IDN-functionality to mimic glibc's IDN behaviour.  I
might even get around to adding IDN-functionality to gnulib, but no
promises.

Any objections?  I'll test the rest of the apps in InetUtils and remove
this code if there aren't any objections.

When modifying code here, I noticed that libping.c had code for
platforms that lack getaddrinfo, but we already use gnulib's
getaddrinfo-replacement.  Thus the patch below also removed that code
too.  I also noticed there is code duplication between ping and ping6,
but I left that alone for now.

/Simon

diff --git a/ping/libping.c b/ping/libping.c
index 5ee33b9f..7f515898 100644
--- a/ping/libping.c
+++ b/ping/libping.c
@@ -33,11 +33,6 @@
 #include <stdio.h>
 #include <errno.h>
 #include <string.h>
-#if defined HAVE_IDN2_H && defined HAVE_IDN2
-# include <idn2.h>
-#elif defined HAVE_IDNA_H
-# include <idna.h>
-#endif
 
 #include "ping.h"
 
@@ -291,19 +286,8 @@ ping_set_packetsize (PING * ping, size_t size)
 int
 ping_set_dest (PING * ping, const char *host)
 {
-#if HAVE_DECL_GETADDRINFO
   int rc;
   struct addrinfo hints, *res;
-  char *rhost;
-
-# if defined HAVE_IDN || defined HAVE_IDN2
-  rc = idna_to_ascii_lz (host, &rhost, 0);     /* RHOST is allocated.  */
-  if (rc)
-    return 1;
-  host = rhost;
-#else
-  rhost = NULL;
-# endif
 
   memset (&hints, 0, sizeof (hints));
   hints.ai_family = AF_INET;
@@ -316,60 +300,16 @@ ping_set_dest (PING * ping, const char *host)
 # endif
 
   rc = getaddrinfo (host, NULL, &hints, &res);
-
   if (rc)
-    {
-      free (rhost);
-      return 1;
-    }
+    return 1;
 
   memcpy (&ping->ping_dest.ping_sockaddr, res->ai_addr, res->ai_addrlen);
   if (res->ai_canonname)
     ping->ping_hostname = strdup (res->ai_canonname);
   else
-# if defined HAVE_IDN || defined HAVE_IDN2
-    ping->ping_hostname = host;
-#else
     ping->ping_hostname = strdup (host);
-# endif
 
   freeaddrinfo (res);
 
   return 0;
-#else /* !HAVE_DECL_GETADDRINFO */
-
-  struct sockaddr_in *s_in = &ping->ping_dest.ping_sockaddr;
-  s_in->sin_family = AF_INET;
-# ifdef HAVE_STRUCT_SOCKADDR_IN_SIN_LEN
-  s_in->sin_len = sizeof (*s_in);
-# endif
-  if (inet_aton (host, &s_in->sin_addr))
-    ping->ping_hostname = strdup (host);
-  else
-    {
-      struct hostent *hp;
-# if defined HAVE_IDN || defined HAVE_IDN2
-      char *rhost;
-      int rc;
-
-      rc = idna_to_ascii_lz (host, &rhost, 0);
-      if (rc)
-       return 1;
-      hp = gethostbyname (rhost);
-      free (rhost);
-# else /* !HAVE_IDN && !HAVE_IDN2 */
-      hp = gethostbyname (host);
-# endif
-      if (!hp)
-       return 1;
-
-      s_in->sin_family = hp->h_addrtype;
-      if (hp->h_length > (int) sizeof (s_in->sin_addr))
-       hp->h_length = sizeof (s_in->sin_addr);
-
-      memcpy (&s_in->sin_addr, hp->h_addr, hp->h_length);
-      ping->ping_hostname = strdup (hp->h_name);
-    }
-  return 0;
-#endif /* !HAVE_DECL_GETADDRINFO */
 }
diff --git a/ping/ping6.c b/ping/ping6.c
index e3d49d6b..0a124c2e 100644
--- a/ping/ping6.c
+++ b/ping/ping6.c
@@ -46,11 +46,6 @@
 #ifdef HAVE_LOCALE_H
 # include <locale.h>
 #endif
-#if defined HAVE_IDN2_H && defined HAVE_IDN2
-# include <idn2.h>
-#elif defined HAVE_IDNA_H
-# include <idna.h>
-#endif
 
 #include <unused-parameter.h>
 #include <xalloc.h>
@@ -1006,16 +1001,6 @@ ping_set_dest (PING * ping, const char *host)
 {
   int err;
   struct addrinfo *result, hints;
-  char *rhost;
-
-#if defined HAVE_IDN || defined HAVE_IDN2
-  err = idna_to_ascii_lz (host, &rhost, 0);
-  if (err)
-    return 1;
-  host = rhost;
-#else /* !HAVE_IDN && !HAVE_IDN2 */
-  rhost = NULL;
-#endif
 
   memset (&hints, 0, sizeof (hints));
   hints.ai_family = AF_INET6;
@@ -1029,10 +1014,7 @@ ping_set_dest (PING * ping, const char *host)
 
   err = getaddrinfo (host, NULL, &hints, &result);
   if (err)
-    {
-      free (rhost);
-      return 1;
-    }
+    return 1;
 
   memcpy (&ping->ping_dest.ping_sockaddr6, result->ai_addr, 
result->ai_addrlen);

signature.asc
Description: PGP signature