gproxyresolver: lookup_finish() should better parallel lookup_async()

In g_proxy_resolver_lookup_async() we have some error validation that
detects invalid URIs and directly returns an error, bypassing the
interface's lookup_async() function. This is great, but when the
interface's lookup_finish() function gets called later, it may assert
that the source tag of the GTask matches the interface's lookup_async()
function, which will not be the case.

As suggested by Philip, we need to check for this situation in
g_proxy_resolver_lookup_finish() and avoid calling into the interface
here if we did the same in g_proxy_resolver_lookup_async(). This can be
done by checking the source tag.

I added a few new tests to check the invalid URI "asdf" used in the
issue report. The final case, using async GProxyResolver directly,
checks for this bug.

Fixes #2799
This commit is contained in:
Michael Catanzaro 2022-11-01 13:35:06 -05:00
parent 993325400d
commit 7f85fd126b
2 changed files with 50 additions and 6 deletions

View File

@ -245,6 +245,9 @@ g_proxy_resolver_lookup_finish (GProxyResolver *resolver,
g_return_val_if_fail (G_IS_PROXY_RESOLVER (resolver), NULL); g_return_val_if_fail (G_IS_PROXY_RESOLVER (resolver), NULL);
if (g_async_result_is_tagged (result, g_proxy_resolver_lookup_async))
return g_task_propagate_pointer (G_TASK (result), error);
iface = G_PROXY_RESOLVER_GET_IFACE (resolver); iface = G_PROXY_RESOLVER_GET_IFACE (resolver);
proxy_uris = (* iface->lookup_finish) (resolver, result, error); proxy_uris = (* iface->lookup_finish) (resolver, result, error);

View File

@ -20,7 +20,6 @@
#include <string.h> #include <string.h>
#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_34
#include <gio/gio.h> #include <gio/gio.h>
/* Overview: /* Overview:
@ -194,6 +193,7 @@ g_test_proxy_resolver_lookup_async (GProxyResolver *resolver,
proxies = g_proxy_resolver_lookup (resolver, uri, cancellable, &error); proxies = g_proxy_resolver_lookup (resolver, uri, cancellable, &error);
task = g_task_new (resolver, NULL, callback, user_data); task = g_task_new (resolver, NULL, callback, user_data);
g_task_set_source_tag (task, g_test_proxy_resolver_lookup_async);
if (proxies == NULL) if (proxies == NULL)
g_task_return_error (task, error); g_task_return_error (task, error);
else else
@ -207,6 +207,9 @@ g_test_proxy_resolver_lookup_finish (GProxyResolver *resolver,
GAsyncResult *result, GAsyncResult *result,
GError **error) GError **error)
{ {
g_assert_true (g_task_is_valid (result, resolver));
g_assert_true (g_task_get_source_tag (G_TASK (result)) == g_test_proxy_resolver_lookup_async);
return g_task_propagate_pointer (G_TASK (result), error); return g_task_propagate_pointer (G_TASK (result), error);
} }
@ -917,6 +920,18 @@ async_got_error (GObject *source,
g_assert (*error != NULL); g_assert (*error != NULL);
} }
static void
async_resolver_got_error (GObject *source,
GAsyncResult *result,
gpointer user_data)
{
GError **error = user_data;
g_assert (error != NULL && *error == NULL);
g_proxy_resolver_lookup_finish (G_PROXY_RESOLVER (source),
result, error);
g_assert (*error != NULL);
}
static void static void
assert_direct (GSocketConnection *conn) assert_direct (GSocketConnection *conn)
@ -1166,13 +1181,24 @@ test_invalid_uris_sync (gpointer fixture,
do_echo_test (conn); do_echo_test (conn);
g_object_unref (conn); g_object_unref (conn);
g_clear_pointer (&last_proxies, g_strfreev); g_clear_pointer (&last_proxies, g_strfreev);
/* Trying to use something that is not a URI at all should fail. */
conn = g_socket_client_connect_to_uri (client, "asdf", 0, NULL, &error);
g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT);
g_clear_error (&error);
g_clear_pointer (&last_proxies, g_strfreev);
/* Should still fail if using GProxyResolver directly. */
g_proxy_resolver_lookup (g_proxy_resolver_get_default (), "asdf", NULL, &error);
g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT);
g_clear_error (&error);
} }
static void static void
test_invalid_uris_async (gpointer fixture, test_invalid_uris_async (gpointer fixture,
gconstpointer user_data) gconstpointer user_data)
{ {
GSocketConnection *conn; GSocketConnection *conn = NULL;
GError *error = NULL; GError *error = NULL;
gchar *uri; gchar *uri;
@ -1204,29 +1230,44 @@ test_invalid_uris_async (gpointer fixture,
* we should succeed. * we should succeed.
*/ */
uri = g_strdup_printf ("invalid-then-simple://127.0.0.1:%u", server.server_port); 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, g_socket_client_connect_to_uri_async (client, uri, 0, NULL,
async_got_conn, &conn); async_got_conn, &conn);
g_free (uri); g_free (uri);
while (conn == NULL) while (conn == NULL)
g_main_context_iteration (NULL, TRUE); g_main_context_iteration (NULL, TRUE);
do_echo_test (conn); do_echo_test (conn);
g_object_unref (conn); g_clear_object (&conn);
g_clear_pointer (&last_proxies, g_strfreev); g_clear_pointer (&last_proxies, g_strfreev);
/* If the proxy resolver returns a valid URI before an invalid URI, /* If the proxy resolver returns a valid URI before an invalid URI,
* we should succeed. * we should succeed.
*/ */
uri = g_strdup_printf ("simple-then-invalid://127.0.0.1:%u", server.server_port); 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, g_socket_client_connect_to_uri_async (client, uri, 0, NULL,
async_got_conn, &conn); async_got_conn, &conn);
g_free (uri); g_free (uri);
while (conn == NULL) while (conn == NULL)
g_main_context_iteration (NULL, TRUE); g_main_context_iteration (NULL, TRUE);
do_echo_test (conn); do_echo_test (conn);
g_object_unref (conn); g_clear_object (&conn);
g_clear_pointer (&last_proxies, g_strfreev); g_clear_pointer (&last_proxies, g_strfreev);
/* Trying to use something that is not a URI at all should fail. */
g_socket_client_connect_to_uri_async (client, "asdf", 0, NULL,
async_got_error, &error);
while (error == NULL)
g_main_context_iteration (NULL, TRUE);
g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT);
g_clear_error (&error);
g_clear_pointer (&last_proxies, g_strfreev);
/* Should still fail if using GProxyResolver directly. */
g_proxy_resolver_lookup_async (g_proxy_resolver_get_default (), "asdf", NULL,
async_resolver_got_error, &error);
while (error == NULL)
g_main_context_iteration (NULL, TRUE);
g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT);
g_clear_error (&error);
} }
static void static void