diff --git a/gio/gproxyaddressenumerator.c b/gio/gproxyaddressenumerator.c index 322f0acae..4e6d58a2d 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" @@ -89,6 +90,20 @@ struct _GProxyAddressEnumeratorPrivate gboolean supports_hostname; GList *next_dest_ip; GError *last_error; + + /* ever_enumerated is TRUE after we've returned a result for the first time + * via g_proxy_address_enumerator_next() or _next_async(). If FALSE, we have + * never returned yet, and should return an error if returning NULL because + * it does not make sense for a proxy resolver to return NULL except on error. + * (Whereas a DNS resolver would return NULL with no error to indicate "no + * results", a proxy resolver would want to return "direct://" instead, so + * NULL without error does not make sense for us.) + * + * But if ever_enumerated is TRUE, then we must not report any further errors + * (except for G_IO_ERROR_CANCELLED), because this is an API contract of + * GSocketAddressEnumerator. + */ + gboolean ever_enumerated; }; G_DEFINE_TYPE_WITH_PRIVATE (GProxyAddressEnumerator, g_proxy_address_enumerator, G_TYPE_SOCKET_ADDRESS_ENUMERATOR) @@ -173,8 +188,9 @@ g_proxy_address_enumerator_next (GSocketAddressEnumerator *enumerator, GSocketAddress *result = NULL; GError *first_error = NULL; - if (priv->proxies == NULL) + if (!priv->ever_enumerated) { + g_assert (priv->proxies == NULL); priv->proxies = g_proxy_resolver_lookup (priv->proxy_resolver, priv->dest_uri, cancellable, @@ -182,7 +198,10 @@ g_proxy_address_enumerator_next (GSocketAddressEnumerator *enumerator, priv->next_proxy = priv->proxies; if (priv->proxies == NULL) - return NULL; + { + priv->ever_enumerated = TRUE; + return NULL; + } } while (result == NULL && (*priv->next_proxy || priv->addr_enum)) @@ -296,29 +315,37 @@ g_proxy_address_enumerator_next (GSocketAddressEnumerator *enumerator, } } - if (result == NULL && first_error) + if (result == NULL && first_error && (!priv->ever_enumerated || g_error_matches (first_error, G_IO_ERROR, G_IO_ERROR_CANCELLED))) g_propagate_error (error, first_error); else if (first_error) g_error_free (first_error); + if (result == NULL && error != 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; } - - static void complete_async (GTask *task) { GProxyAddressEnumeratorPrivate *priv = g_task_get_task_data (task); - if (priv->last_error) + if (priv->last_error && (!priv->ever_enumerated || g_error_matches (priv->last_error, G_IO_ERROR, G_IO_ERROR_CANCELLED))) { 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_clear_error (&priv->last_error); g_object_unref (task); } @@ -392,6 +419,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/gproxyresolver.c b/gio/gproxyresolver.c index 1787bf81f..7ac64df63 100644 --- a/gio/gproxyresolver.c +++ b/gio/gproxyresolver.c @@ -158,6 +158,7 @@ g_proxy_resolver_lookup (GProxyResolver *resolver, GError **error) { GProxyResolverInterface *iface; + gchar **proxy_uris; g_return_val_if_fail (G_IS_PROXY_RESOLVER (resolver), NULL); g_return_val_if_fail (uri != NULL, NULL); @@ -171,7 +172,10 @@ g_proxy_resolver_lookup (GProxyResolver *resolver, iface = G_PROXY_RESOLVER_GET_IFACE (resolver); - return (* iface->lookup) (resolver, uri, cancellable, error); + proxy_uris = (* iface->lookup) (resolver, uri, cancellable, error); + if (proxy_uris == NULL && error != NULL) + g_assert (*error != NULL); + return proxy_uris; } /** @@ -237,10 +241,14 @@ g_proxy_resolver_lookup_finish (GProxyResolver *resolver, GError **error) { GProxyResolverInterface *iface; + gchar **proxy_uris; g_return_val_if_fail (G_IS_PROXY_RESOLVER (resolver), NULL); iface = G_PROXY_RESOLVER_GET_IFACE (resolver); - return (* iface->lookup_finish) (resolver, result, error); + proxy_uris = (* iface->lookup_finish) (resolver, result, error); + if (proxy_uris == NULL && error != NULL) + g_assert (*error != NULL); + return proxy_uris; } diff --git a/gio/gsimpleproxyresolver.c b/gio/gsimpleproxyresolver.c index fdeb6c5a4..8de26cb7b 100644 --- a/gio/gsimpleproxyresolver.c +++ b/gio/gsimpleproxyresolver.c @@ -417,7 +417,7 @@ g_simple_proxy_resolver_class_init (GSimpleProxyResolverClass *resolver_class) object_class->finalize = g_simple_proxy_resolver_finalize; /** - * GSimpleProxyResolver:default-proxy: + * GSimpleProxyResolver:default-proxy: (nullable) * * The default proxy URI that will be used for any URI that doesn't * match #GSimpleProxyResolver:ignore-hosts, and doesn't match any @@ -520,7 +520,7 @@ g_simple_proxy_resolver_new (const gchar *default_proxy, /** * g_simple_proxy_resolver_set_default_proxy: * @resolver: a #GSimpleProxyResolver - * @default_proxy: the default proxy to use + * @default_proxy: (nullable): the default proxy to use * * Sets the default proxy on @resolver, to be used for any URIs that * don't match #GSimpleProxyResolver:ignore-hosts or a proxy set @@ -537,6 +537,7 @@ g_simple_proxy_resolver_set_default_proxy (GSimpleProxyResolver *resolver, const gchar *default_proxy) { g_return_if_fail (G_IS_SIMPLE_PROXY_RESOLVER (resolver)); + g_return_if_fail (default_proxy == NULL || g_uri_is_valid (default_proxy, G_URI_FLAGS_NONE, NULL)); g_free (resolver->priv->default_proxy); resolver->priv->default_proxy = g_strdup (default_proxy); diff --git a/gio/tests/proxy-test.c b/gio/tests/proxy-test.c index 1793006d4..f936a9383 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 - * (GTestAltProxyResolver) always returns [ proxy_a.uri ]. + * and returns [ "direct://" ] for "simple://" URIs, and + * [ proxy_a.uri, proxy_b.uri ] for most other URIs. It can also return + * invalid results for other URIs (empty://, invalid://, + * invalid-then-simple://, and simple-then-invalid://) to test error + * handling. + * + * The other GProxyResolver (GTestAltProxyResolver) always returns + * [ proxy_a.uri ]. */ typedef struct { @@ -131,11 +136,33 @@ g_test_proxy_resolver_lookup (GProxyResolver *resolver, proxies = g_new (gchar *, 3); - if (!strncmp (uri, "simple://", 4)) + if (g_str_has_prefix (uri, "simple://")) { 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 if (g_str_has_prefix (uri, "invalid-then-simple://")) + { + proxies[0] = g_strdup ("😼"); + proxies[1] = g_strdup ("direct://"); + proxies[2] = NULL; + } + else if (g_str_has_prefix (uri, "simple-then-invalid://")) + { + proxies[0] = g_strdup ("direct://"); + proxies[1] = g_strdup ("😼"); + proxies[2] = NULL; + } else { /* Proxy A can only deal with "alpha://" URIs, not @@ -826,11 +853,8 @@ static void teardown_test (gpointer fixture, gconstpointer user_data) { - if (last_proxies) - { - g_strfreev (last_proxies); - last_proxies = NULL; - } + g_clear_pointer (&last_proxies, g_strfreev); + g_clear_error (&proxy_a.last_error); g_clear_error (&proxy_b.last_error); } @@ -1093,6 +1117,118 @@ test_multiple_async (gpointer fixture, g_object_unref (conn); } +static void +test_invalid_uris_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); + g_clear_error (&error); + g_clear_pointer (&last_proxies, g_strfreev); + + /* 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); + g_clear_error (&error); + g_clear_pointer (&last_proxies, g_strfreev); + + /* If the proxy resolver returns an invalid URI before a valid URI, + * we should succeed. + */ + uri = g_strdup_printf ("invalid-then-simple://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_no_error (error); + do_echo_test (conn); + g_object_unref (conn); + g_clear_pointer (&last_proxies, g_strfreev); + + /* If the proxy resolver returns a valid URI before an invalid URI, + * we should succeed. + */ + uri = g_strdup_printf ("simple-then-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_no_error (error); + do_echo_test (conn); + g_object_unref (conn); + g_clear_pointer (&last_proxies, g_strfreev); +} + +static void +test_invalid_uris_async (gpointer fixture, + gconstpointer user_data) +{ + GSocketConnection *conn; + 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); + g_clear_error (&error); + g_clear_pointer (&last_proxies, g_strfreev); + + /* 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); + g_clear_error (&error); + g_clear_pointer (&last_proxies, g_strfreev); + + /* If the proxy resolver returns an invalid URI before a valid URI, + * we should succeed. + */ + uri = g_strdup_printf ("invalid-then-simple://127.0.0.1:%u", server.server_port); + conn = NULL; + g_socket_client_connect_to_uri_async (client, uri, 0, NULL, + async_got_conn, &conn); + g_free (uri); + while (conn == NULL) + g_main_context_iteration (NULL, TRUE); + do_echo_test (conn); + g_object_unref (conn); + g_clear_pointer (&last_proxies, g_strfreev); + + /* If the proxy resolver returns a valid URI before an invalid URI, + * we should succeed. + */ + uri = g_strdup_printf ("simple-then-invalid://127.0.0.1:%u", server.server_port); + conn = NULL; + g_socket_client_connect_to_uri_async (client, uri, 0, NULL, + async_got_conn, &conn); + g_free (uri); + while (conn == NULL) + g_main_context_iteration (NULL, TRUE); + do_echo_test (conn); + g_object_unref (conn); + g_clear_pointer (&last_proxies, g_strfreev); +} + static void test_dns (gpointer fixture, gconstpointer user_data) @@ -1372,6 +1508,8 @@ 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/invalid-uris-sync", 0, NULL, setup_test, test_invalid_uris_sync, teardown_test); + g_test_add_vtable ("/proxy/invalid-uris-async", 0, NULL, setup_test, test_invalid_uris_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);