Merge branch 'th/use-getservbyname-r' into 'main'

gnetworkaddress: use reentrant getservbyname_r() if available

See merge request GNOME/glib!3526

6 files changed, 101 insertions, 14 deletions

diff --git a/gio/gnetworkaddress.c b/gio/gnetworkaddress.c
index 598917de2..a4ee538af 100644
--- a/gio/gnetworkaddress.c
+++ b/gio/gnetworkaddress.c
@@ -486,10 +486,7 @@ g_network_address_parse (const gchar  *host_and_port,
 
       else
         {
-          struct servent *entry;
-
-          entry = getservbyname (port, "tcp");
-          if (entry == NULL)
+          if (!g_getservbyname_ntohs (port, "tcp", &portnum))
             {
               g_set_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT,
                            "Unknown service '%s' specified in hostname '%s'",
@@ -501,8 +498,6 @@ g_network_address_parse (const gchar  *host_and_port,
               return NULL;
             }
 
-          portnum = g_ntohs (entry->s_port);
-
 #ifdef HAVE_ENDSERVENT
           endservent ();
 #endif
diff --git a/gio/gnetworking.c b/gio/gnetworking.c
index fa16238b3..d0c00c98c 100644
--- a/gio/gnetworking.c
+++ b/gio/gnetworking.c
@@ -23,6 +23,7 @@
 #include "config.h"
 
 #include "gnetworking.h"
+#include "gnetworkingprivate.h"
 
 /**
  * SECTION:gnetworking
@@ -76,3 +77,26 @@ g_networking_init (void)
     }
 #endif
 }
+
+gboolean
+g_getservbyname_ntohs (const char *name, const char *proto, guint16 *out_port)
+{
+  struct servent *result;
+
+#ifdef HAVE_GETSERVBYNAME_R
+  struct servent result_buf;
+  char buf[2048];
+  int r;
+
+  r = getservbyname_r (name, proto, &result_buf, buf, sizeof (buf), &result);
+  if (r != 0 || result != &result_buf)
+    result = NULL;
+#else
+  result = getservbyname (name, proto);
+#endif
+
+  if (!result)
+    return FALSE;
+  *out_port = g_ntohs (result->s_port);
+  return TRUE;
+}
diff --git a/gio/gnetworkingprivate.h b/gio/gnetworkingprivate.h
index a212cd862..88894e037 100644
--- a/gio/gnetworkingprivate.h
+++ b/gio/gnetworkingprivate.h
@@ -22,6 +22,7 @@
 #define __G_NETWORKINGPRIVATE_H__
 
 #include "gnetworking.h"
+#include "gresolver.h"
 
 G_BEGIN_DECLS
 
@@ -32,6 +33,8 @@ gint g_socket (gint     domain,
                gint     protocol,
                GError **error);
 
+gboolean g_getservbyname_ntohs (const char *name, const char *proto, guint16 *out_port);
+
 G_END_DECLS
 
 #endif /* __G_NETWORKINGPRIVATE_H__ */
diff --git a/gio/gnetworkservice.c b/gio/gnetworkservice.c
index 8fa74eca3..dac0814d0 100644
--- a/gio/gnetworkservice.c
+++ b/gio/gnetworkservice.c
@@ -364,17 +364,17 @@ static GList *
 g_network_service_fallback_targets (GNetworkService *srv)
 {
   GSrvTarget *target;
-  struct servent *entry;
+  gboolean has_port;
   guint16 port;
 
-  entry = getservbyname (srv->priv->service, "tcp");
-  port = entry ? g_ntohs (entry->s_port) : 0;
+  has_port = g_getservbyname_ntohs (srv->priv->service, "tcp", &port);
+
 #ifdef HAVE_ENDSERVENT
   endservent ();
 #endif
 
-  if (entry == NULL)
-      return NULL;
+  if (!has_port)
+    return NULL;
 
   target = g_srv_target_new (srv->priv->domain, port, 0, 0);
   return g_list_append (NULL, target);
diff --git a/gio/tests/network-address.c b/gio/tests/network-address.c
index da4d7f7e0..61fa41468 100644
--- a/gio/tests/network-address.c
+++ b/gio/tests/network-address.c
@@ -73,10 +73,12 @@ test_parse_uri (gconstpointer d)
     g_error_free (error);
 }
 
-static ParseTest host_tests[] =
-{
+static ParseTest host_tests[] = {
   { "www.gnome.org", NULL, "www.gnome.org", 1234, -1 },
   { "www.gnome.org:8080", NULL, "www.gnome.org", 8080, -1 },
+  { "www.gnome.org:http", NULL, "www.gnome.org", 80, -1 },
+  { "1.2.3.4:imaps", NULL, "1.2.3.4", 993, -1 },
+  { "1.2.3.4:doesnotexist", NULL, NULL, 0, G_IO_ERROR_INVALID_ARGUMENT },
   { "[2001:db8::1]", NULL, "2001:db8::1", 1234, -1 },
   { "[2001:db8::1]:888", NULL, "2001:db8::1", 888, -1 },
   { "[2001:db8::1%em1]", NULL, "2001:db8::1%em1", 1234, -1 },
@@ -85,15 +87,49 @@ static ParseTest host_tests[] =
   { "[hostnam]e", NULL, NULL, 0, G_IO_ERROR_INVALID_ARGUMENT },
   { "hostname:", NULL, NULL, 0, G_IO_ERROR_INVALID_ARGUMENT },
   { "hostname:-1", NULL, NULL, 0, G_IO_ERROR_INVALID_ARGUMENT },
-  { "hostname:9999999", NULL, NULL, 0, G_IO_ERROR_INVALID_ARGUMENT }
+  { "hostname:9999999", NULL, NULL, 0, G_IO_ERROR_INVALID_ARGUMENT },
 };
 
 static void
 test_parse_host (gconstpointer d)
 {
   const ParseTest *test = d;
+  ParseTest test_cpy;
   GNetworkAddress *address;
   GError *error;
+  const char *port_by_name = NULL;
+
+  if (g_str_equal (test->input, "www.gnome.org:http"))
+    port_by_name = "http";
+  else if (g_str_equal (test->input, "1.2.3.4:imaps"))
+    port_by_name = "imaps";
+  else if (g_str_equal (test->input, "1.2.3.4:doesnotexist"))
+    port_by_name = "doesnotexist";
+
+  if (port_by_name)
+    {
+      const struct servent *ent;
+      guint16 port;
+      gint error_code;
+
+      /* If using a named port, check that what’s resolved from the system’s
+       * `/etc/services` matches what’s hard-coded in `host_tests`. */
+
+      ent = getservbyname (port_by_name, "tcp");
+      port = ent ? g_ntohs (ent->s_port) : 0;
+      error_code = ent ? -1 : G_IO_ERROR_INVALID_ARGUMENT;
+
+      if (port != test->port || error_code != test->error_code)
+        {
+          /* We will lookup the port via getservbyname(), but on the
+           * tested system, the result is not as expected. Instead,
+           * adjust our expected test result. */
+          test_cpy = *test;
+          test_cpy.port = port;
+          test_cpy.error_code = error_code;
+          test = &test_cpy;
+        }
+    }
 
   error = NULL;
   address = (GNetworkAddress*)g_network_address_parse (test->input, 1234, &error);
@@ -114,6 +150,17 @@ test_parse_host (gconstpointer d)
     g_object_unref (address);
   if (error)
     g_error_free (error);
+
+  if (test == &test_cpy)
+    {
+      char *msg;
+
+      /* We tested something, but it's not what we originally wanted to test. Mark the
+       * test as skipped. */
+      msg = g_strdup_printf ("getservbyname(\"%s\", \"tcp\") did not give expected result to validate the test", port_by_name);
+      g_test_skip (msg);
+      g_free (msg);
+    }
 }
 
 typedef struct {
diff --git a/meson.build b/meson.build
index bdb2afee4..810c92561 100644
--- a/meson.build
+++ b/meson.build
@@ -756,6 +756,24 @@ if cc.has_function('memalign', prefix: '#include <stdlib.h>\n#include <malloc.h>
   glib_conf.set('HAVE_MEMALIGN', 1)
 endif
 
+# For example on Openbsd, getservbyname_r() has a different signature.
+# https://man.openbsd.org/getservbyname.3
+if cc.compiles('''#include <netdb.h>
+               int main (int argc, char ** argv) {
+                 int (*fcn)(const char *,
+                            const char *,
+                            struct servent *,
+                            char *,
+                            size_t,
+                            struct servent **) = getservbyname_r;
+                 (void) fcn;
+                 return 0;
+               }''',
+               name : 'getservbyname_r()',
+               args: '-Werror=incompatible-pointer-types')
+  glib_conf.set('HAVE_GETSERVBYNAME_R', 1)
+endif
+
 if cc.has_function('_aligned_malloc', prefix: '#include <malloc.h>')
   glib_conf.set('HAVE__ALIGNED_MALLOC', 1)
 endif