gnetworkaddress: Fix incorrect error propagation when resolving addresses

Previously this would always error if ipv6 errored after ipv4
succeeded which was incorrect.

This explicitly tests the order of erroring.

Fixes #1644
This commit is contained in:
Patrick Griffis
2019-02-07 09:50:56 -05:00
committed by Patrick Griffis
parent 0af468e9c9
commit 5b0fdfda6d
2 changed files with 79 additions and 18 deletions

View File

@@ -38,6 +38,10 @@
#include <string.h> #include <string.h>
/* As recommended by RFC 8305 this is the time it waits for a following
DNS response to come in (ipv4 waiting on ipv6 generally)
*/
#define HAPPY_EYEBALLS_RESOLUTION_DELAY_MS 50
/** /**
* SECTION:gnetworkaddress * SECTION:gnetworkaddress
@@ -1143,6 +1147,7 @@ got_ipv6_addresses (GObject *source_object,
GResolver *resolver = G_RESOLVER (source_object); GResolver *resolver = G_RESOLVER (source_object);
GList *addresses; GList *addresses;
GError *error = NULL; GError *error = NULL;
gboolean got_ipv4_first = FALSE;
addresses = g_resolver_lookup_by_name_with_flags_finish (resolver, result, &error); addresses = g_resolver_lookup_by_name_with_flags_finish (resolver, result, &error);
if (!error) if (!error)
@@ -1161,18 +1166,18 @@ got_ipv6_addresses (GObject *source_object,
{ {
g_source_destroy (addr_enum->wait_source); g_source_destroy (addr_enum->wait_source);
g_clear_pointer (&addr_enum->wait_source, g_source_unref); g_clear_pointer (&addr_enum->wait_source, g_source_unref);
got_ipv4_first = TRUE;
} }
/* If we got an error before ipv4 then let it handle it. /* If we got an error before ipv4 then let its response handle it.
* If we get ipv6 response first or error second then * If we get ipv6 response first or error second then
* immediately complete the task. * immediately complete the task.
*/ */
if (error != NULL && !addr_enum->last_error) if (error != NULL && !addr_enum->last_error && !got_ipv4_first)
{ {
addr_enum->last_error = g_steal_pointer (&error); addr_enum->last_error = g_steal_pointer (&error);
/* This shouldn't happen often but avoid never responding. */ addr_enum->wait_source = g_timeout_source_new (HAPPY_EYEBALLS_RESOLUTION_DELAY_MS);
addr_enum->wait_source = g_timeout_source_new_seconds (1);
g_source_set_callback (addr_enum->wait_source, g_source_set_callback (addr_enum->wait_source,
on_address_timeout, on_address_timeout,
addr_enum, NULL); addr_enum, NULL);
@@ -1180,13 +1185,19 @@ got_ipv6_addresses (GObject *source_object,
} }
else if (addr_enum->queued_task != NULL) else if (addr_enum->queued_task != NULL)
{ {
GError *task_error = NULL;
/* If both errored just use the ipv6 one,
but if ipv6 errored and ipv4 didn't we don't error */
if (error != NULL && addr_enum->last_error)
task_error = g_steal_pointer (&error);
g_clear_error (&addr_enum->last_error); g_clear_error (&addr_enum->last_error);
complete_queued_task (addr_enum, g_steal_pointer (&addr_enum->queued_task), complete_queued_task (addr_enum, g_steal_pointer (&addr_enum->queued_task),
g_steal_pointer (&error)); g_steal_pointer (&task_error));
} }
else if (error != NULL)
g_clear_error (&error);
g_clear_error (&error);
g_object_unref (addr_enum); g_object_unref (addr_enum);
} }
@@ -1229,7 +1240,7 @@ got_ipv4_addresses (GObject *source_object,
else if (addr_enum->queued_task != NULL) else if (addr_enum->queued_task != NULL)
{ {
addr_enum->last_error = g_steal_pointer (&error); addr_enum->last_error = g_steal_pointer (&error);
addr_enum->wait_source = g_timeout_source_new (50); addr_enum->wait_source = g_timeout_source_new (HAPPY_EYEBALLS_RESOLUTION_DELAY_MS);
g_source_set_callback (addr_enum->wait_source, g_source_set_callback (addr_enum->wait_source,
on_address_timeout, on_address_timeout,
addr_enum, NULL); addr_enum, NULL);

View File

@@ -710,17 +710,18 @@ test_happy_eyeballs_slow_connection_and_ipv4 (HappyEyeballsFixture *fixture,
} }
static void static void
test_happy_eyeballs_ipv6_error (HappyEyeballsFixture *fixture, test_happy_eyeballs_ipv6_error_ipv4_first (HappyEyeballsFixture *fixture,
gconstpointer user_data) gconstpointer user_data)
{ {
AsyncData data = { 0 }; AsyncData data = { 0 };
GError *ipv6_error; GError *ipv6_error;
/* If ipv6 fails we still get ipv4. */ /* If ipv6 fails, ensuring that ipv4 finishes before ipv6 errors, we still get ipv4. */
data.loop = fixture->loop; data.loop = fixture->loop;
ipv6_error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_TIMED_OUT, "IPv6 Broken"); ipv6_error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_TIMED_OUT, "IPv6 Broken");
mock_resolver_set_ipv6_error (fixture->mock_resolver, ipv6_error); mock_resolver_set_ipv6_error (fixture->mock_resolver, ipv6_error);
mock_resolver_set_ipv6_delay_ms (fixture->mock_resolver, 25);
g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data); g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data);
g_main_loop_run (fixture->loop); g_main_loop_run (fixture->loop);
@@ -731,17 +732,62 @@ test_happy_eyeballs_ipv6_error (HappyEyeballsFixture *fixture,
} }
static void static void
test_happy_eyeballs_ipv4_error (HappyEyeballsFixture *fixture, test_happy_eyeballs_ipv6_error_ipv6_first (HappyEyeballsFixture *fixture,
gconstpointer user_data) gconstpointer user_data)
{
AsyncData data = { 0 };
GError *ipv6_error;
/* If ipv6 fails, ensuring that ipv6 errors before ipv4 finishes, we still get ipv4. */
data.loop = fixture->loop;
ipv6_error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_TIMED_OUT, "IPv6 Broken");
mock_resolver_set_ipv6_error (fixture->mock_resolver, ipv6_error);
mock_resolver_set_ipv4_delay_ms (fixture->mock_resolver, 25);
g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data);
g_main_loop_run (fixture->loop);
assert_list_matches_expected (data.addrs, fixture->input_ipv4_results);
g_error_free (ipv6_error);
}
static void
test_happy_eyeballs_ipv4_error_ipv4_first (HappyEyeballsFixture *fixture,
gconstpointer user_data)
{ {
AsyncData data = { 0 }; AsyncData data = { 0 };
GError *ipv4_error; GError *ipv4_error;
/* If ipv4 fails we still get ipv6. */ /* If ipv4 fails, ensuring that ipv4 errors before ipv6 finishes, we still get ipv6. */
data.loop = fixture->loop; data.loop = fixture->loop;
ipv4_error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_TIMED_OUT, "IPv4 Broken"); ipv4_error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_TIMED_OUT, "IPv4 Broken");
mock_resolver_set_ipv4_error (fixture->mock_resolver, ipv4_error); mock_resolver_set_ipv4_error (fixture->mock_resolver, ipv4_error);
mock_resolver_set_ipv6_delay_ms (fixture->mock_resolver, 25);
g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data);
g_main_loop_run (fixture->loop);
assert_list_matches_expected (data.addrs, fixture->input_ipv6_results);
g_error_free (ipv4_error);
}
static void
test_happy_eyeballs_ipv4_error_ipv6_first (HappyEyeballsFixture *fixture,
gconstpointer user_data)
{
AsyncData data = { 0 };
GError *ipv4_error;
/* If ipv4 fails, ensuring that ipv6 finishes before ipv4 errors, we still get ipv6. */
data.loop = fixture->loop;
ipv4_error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_TIMED_OUT, "IPv4 Broken");
mock_resolver_set_ipv4_error (fixture->mock_resolver, ipv4_error);
mock_resolver_set_ipv4_delay_ms (fixture->mock_resolver, 25);
g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data); g_socket_address_enumerator_next_async (fixture->enumerator, NULL, got_addr, &data);
g_main_loop_run (fixture->loop); g_main_loop_run (fixture->loop);
@@ -913,10 +959,14 @@ main (int argc, char *argv[])
happy_eyeballs_setup, test_happy_eyeballs_very_slow_ipv6, happy_eyeballs_teardown); happy_eyeballs_setup, test_happy_eyeballs_very_slow_ipv6, happy_eyeballs_teardown);
g_test_add ("/network-address/happy-eyeballs/slow-connection-and-ipv4", HappyEyeballsFixture, NULL, g_test_add ("/network-address/happy-eyeballs/slow-connection-and-ipv4", HappyEyeballsFixture, NULL,
happy_eyeballs_setup, test_happy_eyeballs_slow_connection_and_ipv4, happy_eyeballs_teardown); happy_eyeballs_setup, test_happy_eyeballs_slow_connection_and_ipv4, happy_eyeballs_teardown);
g_test_add ("/network-address/happy-eyeballs/ipv6-error", HappyEyeballsFixture, NULL, g_test_add ("/network-address/happy-eyeballs/ipv6-error-ipv4-first", HappyEyeballsFixture, NULL,
happy_eyeballs_setup, test_happy_eyeballs_ipv6_error, happy_eyeballs_teardown); happy_eyeballs_setup, test_happy_eyeballs_ipv6_error_ipv4_first, happy_eyeballs_teardown);
g_test_add ("/network-address/happy-eyeballs/ipv4-error", HappyEyeballsFixture, NULL, g_test_add ("/network-address/happy-eyeballs/ipv6-error-ipv6-first", HappyEyeballsFixture, NULL,
happy_eyeballs_setup, test_happy_eyeballs_ipv4_error, happy_eyeballs_teardown); happy_eyeballs_setup, test_happy_eyeballs_ipv6_error_ipv6_first, happy_eyeballs_teardown);
g_test_add ("/network-address/happy-eyeballs/ipv4-error-ipv6-first", HappyEyeballsFixture, NULL,
happy_eyeballs_setup, test_happy_eyeballs_ipv4_error_ipv6_first, happy_eyeballs_teardown);
g_test_add ("/network-address/happy-eyeballs/ipv4-error-ipv4-first", HappyEyeballsFixture, NULL,
happy_eyeballs_setup, test_happy_eyeballs_ipv4_error_ipv4_first, happy_eyeballs_teardown);
g_test_add ("/network-address/happy-eyeballs/both-error", HappyEyeballsFixture, NULL, g_test_add ("/network-address/happy-eyeballs/both-error", HappyEyeballsFixture, NULL,
happy_eyeballs_setup, test_happy_eyeballs_both_error, happy_eyeballs_teardown); happy_eyeballs_setup, test_happy_eyeballs_both_error, happy_eyeballs_teardown);
g_test_add ("/network-address/happy-eyeballs/both-error-delays-1", HappyEyeballsFixture, NULL, g_test_add ("/network-address/happy-eyeballs/both-error-delays-1", HappyEyeballsFixture, NULL,