[glib/mcatanzaro/#2597: 13/15] proxyaddressenumerator: always set error on first enumeration

[Michael Catanzaro <mcatanzaro src gnome org>]

commit 7b573b7f60d281f78f6dd0a65e985680f4fd1463
Author: Michael Catanzaro <mcatanzaro redhat com>
Date:   Thu Jun 9 13:30:02 2022 -0500

    proxyaddressenumerator: always set error on first enumeration
    
    The documentation implies that an error will be returned if the first
    enumeration returns NULL. Let's make sure this actually happens by
    adding some fallback errors to return in case GProxyResolver feeds us
    weird results.
    
    Fixes #2597

 gio/gproxyaddressenumerator.c |  18 +++++++-
 gio/tests/proxy-test.c        | 100 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 115 insertions(+), 3 deletions(-)
---
diff --git a/gio/gproxyaddressenumerator.c b/gio/gproxyaddressenumerator.c
index 322f0acaee..0701ae245b 100644
--- a/gio/gproxyaddressenumerator.c
+++ b/gio/gproxyaddressenumerator.c
@@ -27,6 +27,7 @@
 
 #include "gasyncresult.h"
 #include "ginetaddress.h"
+#include "gioerror.h"
 #include "glibintl.h"
 #include "gnetworkaddress.h"
 #include "gnetworkingprivate.h"
@@ -87,6 +88,7 @@ struct _GProxyAddressEnumeratorPrivate
   gchar                    *proxy_username;
   gchar                    *proxy_password;
   gboolean                  supports_hostname;
+  gboolean                  ever_enumerated;
   GList                    *next_dest_ip;
   GError                   *last_error;
 };
@@ -182,7 +184,11 @@ g_proxy_address_enumerator_next (GSocketAddressEnumerator  *enumerator,
       priv->next_proxy = priv->proxies;
 
       if (priv->proxies == NULL)
-       return NULL;
+       {
+         priv->ever_enumerated = TRUE;
+         g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, _("Unspecified proxy lookup failure"));
+         return NULL;
+       }
     }
 
   while (result == NULL && (*priv->next_proxy || priv->addr_enum))
@@ -301,6 +307,11 @@ g_proxy_address_enumerator_next (GSocketAddressEnumerator  *enumerator,
   else if (first_error)
     g_error_free (first_error);
 
+  if (result == NULL && *error == NULL && !priv->ever_enumerated)
+    g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, _("Unspecified proxy lookup failure"));
+
+  priv->ever_enumerated = TRUE;
+
   return result;
 }
 
@@ -316,9 +327,13 @@ complete_async (GTask *task)
       g_task_return_error (task, priv->last_error);
       priv->last_error = NULL;
     }
+  else if (!priv->ever_enumerated)
+    g_task_return_new_error (task, G_IO_ERROR, G_IO_ERROR_FAILED, _("Unspecified proxy lookup failure"));
   else
     g_task_return_pointer (task, NULL, NULL);
 
+  priv->ever_enumerated = TRUE;
+
   g_object_unref (task);
 }
 
@@ -392,6 +407,7 @@ return_result (GTask *task)
        }
     }
 
+  priv->ever_enumerated = TRUE;
   g_task_return_pointer (task, result, g_object_unref);
   g_object_unref (task);
 }
diff --git a/gio/tests/proxy-test.c b/gio/tests/proxy-test.c
index ed0111def1..08e1b31ab5 100644
--- a/gio/tests/proxy-test.c
+++ b/gio/tests/proxy-test.c
@@ -43,9 +43,14 @@
  * connects to @server_addr anyway).
  *
  * The default GProxyResolver (GTestProxyResolver) looks at its URI
- * and returns [ "direct://" ] for "simple://" URIs, and [
- * proxy_a.uri, proxy_b.uri ] for other URIs. The other GProxyResolver
+ * and returns [ "direct://" ] for "simple://" URIs, an empty string
+ * for "empty://" URIs, an invalid URI for "invalid://" URIs, and
+ * [ proxy_a.uri, proxy_b.uri ] for other URIs. The other GProxyResolver
  * (GTestAltProxyResolver) always returns [ proxy_a.uri ].
+ *
+ * Some of these URIs (for `empty:` and `invalid:` proxy URIs)
+ * are deliberately invalid, to test error handling further down
+ * the pipeline.
  */
 
 typedef struct {
@@ -136,6 +141,16 @@ g_test_proxy_resolver_lookup (GProxyResolver  *resolver,
       proxies[0] = g_strdup ("direct://");
       proxies[1] = NULL;
     }
+  else if (g_str_has_prefix (uri, "empty://"))
+    {
+      proxies[0] = g_strdup ("");
+      proxies[1] = NULL;
+    }
+  else if (g_str_has_prefix (uri, "invalid://"))
+    {
+      proxies[0] = g_strdup ("😼");
+      proxies[1] = NULL;
+    }
   else
     {
       /* Proxy A can only deal with "alpha://" URIs, not
@@ -1093,6 +1108,83 @@ test_multiple_async (gpointer fixture,
   g_object_unref (conn);
 }
 
+static void
+test_empty_string_sync (gpointer fixture,
+                       gconstpointer user_data)
+{
+  GSocketConnection *conn;
+  gchar *uri;
+  GError *error = NULL;
+
+  g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2597";);
+
+  /* The empty:// URI causes the proxy resolver to return an empty string. */
+  uri = g_strdup_printf ("empty://127.0.0.1:%u", server.server_port);
+  conn = g_socket_client_connect_to_uri (client, uri, 0, NULL, &error);
+  g_free (uri);
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_FAILED);
+  g_assert_null (conn);
+}
+
+static void
+test_empty_string_async (gpointer fixture,
+                        gconstpointer user_data)
+{
+  GError *error = NULL;
+  gchar *uri;
+
+  g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2597";);
+
+  /* The empty:// URI causes the proxy resolver to return an empty string. */
+  uri = g_strdup_printf ("empty://127.0.0.1:%u", server.server_port);
+  g_socket_client_connect_to_uri_async (client, uri, 0, NULL,
+                                       async_got_error, &error);
+  g_free (uri);
+  while (error == NULL)
+    g_main_context_iteration (NULL, TRUE);
+
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_FAILED);
+}
+
+static void
+test_invalid_uri_sync (gpointer fixture,
+                      gconstpointer user_data)
+{
+  GSocketConnection *conn;
+  gchar *uri;
+  GError *error = NULL;
+
+  g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2597";);
+
+  /* The invalid:// URI causes the proxy resolver to return a cat emoji. */
+  uri = g_strdup_printf ("invalid://127.0.0.1:%u", server.server_port);
+  conn = g_socket_client_connect_to_uri (client, uri, 0, NULL, &error);
+  g_free (uri);
+
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_FAILED);
+  g_assert_null (conn);
+}
+
+static void
+test_invalid_uri_async (gpointer fixture,
+                       gconstpointer user_data)
+{
+  GError *error = NULL;
+  gchar *uri;
+
+  g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2597";);
+
+  /* The invalid:// URI causes the proxy resolver to return a cat emoji. */
+  uri = g_strdup_printf ("invalid://127.0.0.1:%u", server.server_port);
+  g_socket_client_connect_to_uri_async (client, uri, 0, NULL,
+                                       async_got_error, &error);
+  g_free (uri);
+  while (error == NULL)
+    g_main_context_iteration (NULL, TRUE);
+
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_FAILED);
+}
+
 static void
 test_dns (gpointer fixture,
          gconstpointer user_data)
@@ -1372,6 +1464,10 @@ main (int   argc,
   g_test_add_vtable ("/proxy/single_async", 0, NULL, setup_test, test_single_async, teardown_test);
   g_test_add_vtable ("/proxy/multiple_sync", 0, NULL, setup_test, test_multiple_sync, teardown_test);
   g_test_add_vtable ("/proxy/multiple_async", 0, NULL, setup_test, test_multiple_async, teardown_test);
+  g_test_add_vtable ("/proxy/empty-string-sync", 0, NULL, setup_test, test_empty_string_sync, teardown_test);
+  g_test_add_vtable ("/proxy/empty-string-async", 0, NULL, setup_test, test_empty_string_async, 
teardown_test);
+  g_test_add_vtable ("/proxy/invalid-uri-sync", 0, NULL, setup_test, test_invalid_uri_sync, teardown_test);
+  g_test_add_vtable ("/proxy/invalid-uri-async", 0, NULL, setup_test, test_invalid_uri_async, teardown_test);
   g_test_add_vtable ("/proxy/dns", 0, NULL, setup_test, test_dns, teardown_test);
   g_test_add_vtable ("/proxy/override", 0, NULL, setup_test, test_override, teardown_test);
   g_test_add_func ("/proxy/enumerator-ports", test_proxy_enumerator_ports);