From f0a7b147806e852e2090eeda6e4e38f7d3f52b52 Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Tue, 6 Oct 2020 15:39:45 -0500 Subject: [PATCH] gsocketclient: emit RESOLVING/RESOLVED events only once GSocketAddressEnumerator encapsulates the details of how DNS happens, so we don't have to think about it. But we may have taken encapsulation a bit too far, here. Usually, we resolve a domain name to a list of IPv4 and IPv6 addresses. Then we go through each address in the list and try to connect to it. Name resolution happens exactly once, at the start. It doesn't happen each time we enumerate the enumerator. In theory, it *could*, because we've designed these APIs to be agnostic of underlying implementation details like DNS and network protocols. But in practice, we know that's not really what's happening. It's weird to say that we are RESOLVING what we know to be the same name multiple times. Behind the scenes, we're not doing that. This also fixes #1994, where enumeration can end with a RESOLVING event, even though this is supposed to be the first event rather than the last. I thought this would be hard to fix, even requiring new public API in GSocketAddressEnumerator to peek ahead to see if the next enumeration is going to return NULL. Then I decided we should just fake it: always emit both RESOLVING and RESOLVED at the same time right after each enumeration. Finally, I realized we can emit them at the correct time if we simply assume resolving only happens the first time. This seems like the most elegant of the possible solutions. Now, this is a behavior change, and arguably an API break, but it should align better with reasonable expectations of how GSocketClientEvent ought to work. I don't expect it to break anything besides tests that check which order GSocketClientEvent events are emitted in. (Currently, libsoup has such tests, which will need to be updated.) Ideally we would have GLib-level tests as well, but in a concession to pragmatism, it's a lot easier to keep network tests in libsoup. --- gio/gsocketclient.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/gio/gsocketclient.c b/gio/gsocketclient.c index 8a663c3de..9df8f290f 100644 --- a/gio/gsocketclient.c +++ b/gio/gsocketclient.c @@ -991,6 +991,7 @@ g_socket_client_connect (GSocketClient *client, { GIOStream *connection = NULL; GSocketAddressEnumerator *enumerator = NULL; + gboolean ever_resolved = FALSE; GError *last_error, *tmp_error; last_error = NULL; @@ -1025,10 +1026,20 @@ g_socket_client_connect (GSocketClient *client, } tmp_error = NULL; - g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVING, - connectable, NULL); + + if (!ever_resolved) + { + g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVING, + connectable, NULL); + } address = g_socket_address_enumerator_next (enumerator, cancellable, &tmp_error); + if (!ever_resolved) + { + g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVED, + connectable, NULL); + ever_resolved = TRUE; + } if (address == NULL) { @@ -1046,8 +1057,6 @@ g_socket_client_connect (GSocketClient *client, _("Unknown error on connect")); break; } - g_socket_client_emit_event (client, G_SOCKET_CLIENT_RESOLVED, - connectable, NULL); using_proxy = (G_IS_PROXY_ADDRESS (address) && client->priv->enable_proxy); @@ -1509,7 +1518,8 @@ enumerator_next_async (GSocketClientAsyncConnectData *data, if (add_task_ref) g_object_ref (data->task); - g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVING, data->connectable, NULL); + if (!data->enumerated_at_least_once) + g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVING, data->connectable, NULL); g_debug ("GSocketClient: Starting new address enumeration"); g_socket_address_enumerator_next_async (data->enumerator, data->enumeration_cancellable, @@ -1883,10 +1893,13 @@ g_socket_client_enumerator_callback (GObject *object, return; } - data->enumerated_at_least_once = TRUE; g_debug ("GSocketClient: Address enumeration succeeded"); - g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVED, - data->connectable, NULL); + if (!data->enumerated_at_least_once) + { + g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVED, + data->connectable, NULL); + data->enumerated_at_least_once = TRUE; + } g_clear_error (&data->last_error);